Re: [PATCH 02/27] drivers/net: fix sparse warnings: make do-while a compound statement

[Krzysztof Halasa]

Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> writes:

> It's more than that. I added the check after some person who had been 
> programming the kernel (and thus was supposedly fluent in C) literally 
> could not parse a macro that had "do x while (y)" in it.

A literally that simple macro, or a complicated one?


I'm all for using brackets when there is / could be some possibility
of increasing code readability.

E.g. I always use parentheses in a nested if-if, because

	if (x)
		if (y)
			a;
		else
			c;

may be confusing, especially if formatted differently.

	if (x)
		a;
	else if (y)
		b;
	etc.

is simple and unambiguous and I don't put the braces.

So is a case like
	do
		x;
	while (y);
It can't be made more clear with brackets.

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

> Another example of this is "sizeof". The kernel universally (I hope) has 
> parenthesis around the sizeof argument, even though it's clearly not 
> required by the C language. 
>
> It's a coding standard.

Right, but they (at least for me) make it more readable.
kmalloc(sizeof i) just doesn't look good, the operator looks like
a variable name.

But there is this return statement. Some people tend to write
return (x); I simply write return x;
It's clear, and so is a simple do-while.

> And quite frankly, anybody who works on gcc has no place complaining about 
> sparse coding standard warnings. They are a _hell_ of a lot better than 
> some of the really crazy warnings gcc spews out with "-W". At least the 
> sparse warnings you can make go away while making the code more 
> understandable. Some of the -W warnings are unfixable without breaking the 
> source code.

:-)

BTW I think I may use sparse differently.
I can see false gcc warnings every time the project is being built.
OTOH I run sparse only when I have some (almost) completed project
(a patch, a driver etc). I make sure the remaining sparse warnings are
(from my POV) invalid and it won't spew them on next build again.
-- 
Krzysztof Halasa
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/