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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 24 Dec 2008 00:18:46 +0100
From:	Krzysztof Halasa <khc@...waw.pl>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
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

Linus Torvalds <torvalds@...ux-foundation.org> 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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/