Message-ID: <alpine.LFD.2.00.0812231529030.3535@localhost.localdomain>
Date:	Tue, 23 Dec 2008 15:38:02 -0800 (PST)
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Krzysztof Halasa <khc@...waw.pl>
cc:	Harvey Harrison <harvey.harrison@...il.com>,
	Håkon Løvdal <hlovdal@...il.com>,
	Hannes Eder <hannes@...neseder.net>, netdev@...r.kernel.org,
	kernel-janitors@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 02/27] drivers/net: fix sparse warnings: make do-while
 a compound statement



On Wed, 24 Dec 2008, Krzysztof Halasa wrote:
> 
> So is a case like
> 	do
> 		x;
> 	while (y);
> It can't be made more clear with brackets.

Oh yes it can. People look at that, and it's so uncommon that they 
literally believe it is a mis-indent.

Your example with nested if-statements are totally pointless, because you 
didn't even apparently understand my comment about "while()" having two 
totally different meanings (which is not true of "if()"), nor realize the 
importance of how common something is.

Common patterns become things that people take for granted and don't have 
any trouble with. In contrast, do-while without braces is _extremely_ 
uncommon.

> IOW: improving the style is great. Changing it only to silence some
> tool is not.

Sorry, you're wrong. It's not changed to silence some tool. THIS IS THE 
KERNEL CODING STYLE. 

I don't care one whit about your personal coding style. The kernel has 
brances. End of discussion. sparse complains about lack of them. 
Comprende?

> Right, but they (at least for me) make it more readable.

I don't care.

> kmalloc(sizeof i) just doesn't look good, the operator looks like
> a variable name.

And I agree with you. "sizeof i" doesn't look good. It's uncommon, and 
doesn't match peoples expectations.

> But there is this return statement. Some people tend to write
> return (x); I simply write return x;

I do to. And it's the common thing to do, and only totally confused people 
think that 'return' is a somehow remotely like a "function" of its 
arguments (the way 'sizeof' is - sizeof _is_ a function of its arguments, 
albeit a very rare one).

> It's clear, and so is a simple do-while.

No. "return x;" is clear because it's the common thing, which means that 
peopel are good at reading it.

Another example of "common vs non-common" is this:

	if (0 <= x)
		do something..

is something that crazy people do (sadly, one of the crazy people taught 
the git maintainer C programming, so now even sane people do it). It's 
crazy because it's uncommon, which means that most people have to think 
about it A LOT MORE than about

	if (x >= 0)
		do something..

even though technically both are obviously EXACTLY THE SAME THING.

Can you see the argument? Doing things the common way is important, 
because it allows people to see what they mean without having to think 
about it. They just scan it, and the meaning is clear.

And that's why "do while" without braces is bad. If you scan it quickly on 
its own, you may well end up just seeing the

	while (x);

part, and get confused ("oh, a delay loop"). But if you see

	} while (x);

you aren't confused, because the latter one is clearly an ending condition 
of a do-while loop, IN A WAY THAT THE FIRST ONE IS NOT!

See?

do-while is very special, because as mentioned, "while" is a really magic 
C keyword that has two TOTALLY DIFFERENT meanings. Don't make people look 
for the "do".

		Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists