linux-kernel - Re: [PATCH] Don't compare unsigned variable for <0 in sys

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]

Message-ID: <Pine.LNX.4.64.0611281600020.4244@woody.osdl.org>
Date:	Tue, 28 Nov 2006 16:13:05 -0800 (PST)
From:	Linus Torvalds <torvalds@...l.org>
To:	Jesper Juhl <jesper.juhl@...il.com>
cc:	linux-kernel@...r.kernel.org, Andrew Morton <akpm@...l.org>,
	trivial@...nel.org
Subject: Re: [PATCH] Don't compare unsigned variable for <0 in sys_prctl()

On Wed, 29 Nov 2006, Jesper Juhl wrote:
> 
> I would venture that "-Wshadow" is another one of those. 

I'd agree, except for the fact that gcc does a horribly _bad_ job of 
-Wshadow, making it (again) totally unusable.

For example, it's often entirely interesting to hear about local variables 
that shadow each other. No question about it.

HOWEVER. It's _not_ really interesting to hear about a local variable that 
happens to have a common name that is also shared by a extern function. 

There just isn't any room for confusion, and it's actually not even that 
unusual - I tried using -Wshadow on real programs, and it was just 
horribly irritating.

In the kernel, we had obvious things like local use of "jiffies" that just 
make _total_ sense in a small inline function, and the fact that there 
happens to be an extern declaration for "jiffies" just isn't very 
interesting.

Similarly, with nested macro expansion, even the "local variable shadows 
another local variable" case - that looks like it should have an obvious 
warning on the face of it - really isn't always necessarily that 
interesting after all. Maybe it is a bug, maybe it isn't, but it's no 
longer _obviously_ bogus any more.

So I'm not convinced about the usefulness of "-Wshadow". ESPECIALLY the 
way that gcc implements it, it's almost totally useless in real life.

For example, I tried it on "git" one time, and this is a perfect example 
of why "-Wshadow" is totally broken:

	diff-delta.c: In function 'create_delta_index':
	diff-delta.c:142: warning: declaration of 'index' shadows a global declaration

(and there's a _lot_ of those). If I'm not allowed to use "index" as a 
local variable and include <string.h> at the same time, something is 
simply SERIOUSLY WRONG with the warning.

So the fact is, the C language has scoping rules for a reason. Can you 
screw yourself by usign them badly? Sure. But that does NOT mean that the 
same name in different scopes is a bad thing that should be warned about.

If I wanted a language that didn't allow me to do anything wrong, I'd be 
using Pascal. As it is, it turns out that things that "look" wrong on a 
local level are often not wrong after all.

			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/