login
Search
Linux creator Linus Torvalds proclaimed, "friends don't let friends use '-W'," in a thread discussing GCC's handling of warnings. The thread began with a patch to remove an unnecessary comparison of an unsigned variable which resulted in a GCC warning, "comparison of unsigned expression < 0 is always false". Linus suggested it is better to write readable and obvious code, "without having to carry stupid context around in our heads". He went on to add, "if the compiler (whose _job_ it is to carry all that context and use it to generate good code) notices that the fact that 'x' is unsigned means that one of the tests is unnecessary, that does not make it wrong." As has been noted in several earlier lkml discussions [story], Linus concluded, "Gcc warns for a lot of wrong things. This is one of them."
Later in the thread, Linus pointed out that GCC also offers plenty of useful warnings as well. He suggested that while the reason many warnings which are included in '-W' are not included in '-Wall' is because they are useless, there are flags such as '-Wstrict-prototypes' which offer useful warnings and are not included in '-Wall'. When '-Wshadow' was suggested as a useful compiler flag, Linus replied, "I'd agree, except for the fact that gcc does a horribly _bad_ job of -Wshadow, making it (again) totally unusable." He offered an example, "it's often entirely interesting to hear about local variables that shadow each other," then noting, "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." Linus concluded, "the C language has scoping rules for a reason. 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."
From: Jesper Juhl [email blocked]
To: linux-kernel
Subject: [PATCH] Don't compare unsigned variable for <0 in sys_prctl()
Date: Tue, 28 Nov 2006 23:17:13 +0100
In kernel/sys.c::sys_prctl() the argument named 'arg2' is very clearly
of type 'unsigned long', and when compiling with "gcc -W" gcc also warns :
kernel/sys.c:2089: warning: comparison of unsigned expression < 0 is always false
So this patch removes the test of "arg2 < 0".
For those of us who compile their kernels with "-W" this gets rid of an
annoying warning. For the rest of you it saves a few bytes of source code ;-)
Signed-off-by: Jesper Juhl [email blocked]
---
diff --git a/kernel/sys.c b/kernel/sys.c
index 98489d8..086ea37 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -2086,7 +2086,7 @@ asmlinkage long sys_prctl(int option, un
error = current->mm->dumpable;
break;
case PR_SET_DUMPABLE:
- if (arg2 < 0 || arg2 > 1) {
+ if (arg2 > 1) {
error = -EINVAL;
break;
}
From: Linus Torvalds [email blocked]
Subject: Re: [PATCH] Don't compare unsigned variable for <0 in sys_prctl()
Date: Tue, 28 Nov 2006 14:27:51 -0800 (PST)
On Tue, 28 Nov 2006, Jesper Juhl wrote:
>
> In kernel/sys.c::sys_prctl() the argument named 'arg2' is very clearly
> of type 'unsigned long', and when compiling with "gcc -W" gcc also warns :
> kernel/sys.c:2089: warning: comparison of unsigned expression < 0 is always false
>
> So this patch removes the test of "arg2 < 0".
No, we don't do this.
This is why we don't compile with "-W". Gcc is crap.
The fact is, if it's unsigned, it's not something that the programmer
should have to care about. We should write our code to be readable and
obviously safe, and that means that
if (x < 0 || x > MAX)
return -ERROR;
is the _right_ way to do things, without having to carry stupid context
around in our heads.
If the compiler (whose _job_ it is to carry all that context and use it to
generate good code) notices that the fact that "x" is unsignes means that
one of the tests is unnecessary, that does not make it wrong.
Gcc warns for a lot of wrong things. This is one of them.
Friends don't let friends use "-W".
Linus
From: "Jesper Juhl" [email blocked]
Subject: Re: [PATCH] Don't compare unsigned variable for <0 in sys_prctl()
Date: Tue, 28 Nov 2006 23:34:27 +0100
On 28/11/06, Linus Torvalds [email blocked] wrote:
>
> Friends don't let friends use "-W".
>
Hehe, ok, I'll stop cleaning this stuff up then.
Nice little hobby out the window there ;)
--
Jesper Juhl [email blocked]
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please http://www.expita.com/nomime.html
From: Linus Torvalds [email blocked]
Subject: Re: [PATCH] Don't compare unsigned variable for <0 in sys_prctl()
Date: Tue, 28 Nov 2006 15:06:35 -0800 (PST)
On Tue, 28 Nov 2006, Jesper Juhl wrote:
>
> > Friends don't let friends use "-W".
>
> Hehe, ok, I'll stop cleaning this stuff up then.
> Nice little hobby out the window there ;)
You might want to look at some of the other warnings gcc spits out, but
this class isn't one of them.
Other warnings we have added over the years (and that really _are_ good
warnings) have included the "-Wstrict-prototypes", and some other ones.
If you can pinpoint _which_ gcc warning flag it is that causes gcc to emit
the bogus ones, you _could_ try "-W -Wno-xyz-warning", which should cause
gcc to enable all the "other" warnings, but then not the "xyz-warning"
that causes problems.
Of course, there is often a reason why a warning is in "-W" but not in
"-Wall". Most of the time it's sign that the warning is bogus. Not always,
though - we do tend to want to be fairly strict, and Wstrict-prototypes is
an example of a _good_ warning that is not in -Wall.
Linus
From: "Jesper Juhl" [email blocked]
Subject: Re: [PATCH] Don't compare unsigned variable for <0 in sys_prctl()
Date: Wed, 29 Nov 2006 00:42:28 +0100
On 29/11/06, Linus Torvalds [email blocked] wrote:
>
>
> On Tue, 28 Nov 2006, Jesper Juhl wrote:
> >
> > > Friends don't let friends use "-W".
> >
> > Hehe, ok, I'll stop cleaning this stuff up then.
> > Nice little hobby out the window there ;)
>
> You might want to look at some of the other warnings gcc spits out, but
> this class isn't one of them.
>
> Other warnings we have added over the years (and that really _are_ good
> warnings) have included the "-Wstrict-prototypes", and some other ones.
>
> If you can pinpoint _which_ gcc warning flag it is that causes gcc to emit
> the bogus ones, you _could_ try "-W -Wno-xyz-warning", which should cause
> gcc to enable all the "other" warnings, but then not the "xyz-warning"
> that causes problems.
>
> Of course, there is often a reason why a warning is in "-W" but not in
> "-Wall". Most of the time it's sign that the warning is bogus. Not always,
> though - we do tend to want to be fairly strict, and Wstrict-prototypes is
> an example of a _good_ warning that is not in -Wall.
>
I would venture that "-Wshadow" is another one of those. I've, in the
past, submitted quite a few patches to clean up shadow warnings (some
accepted, some not) and I'll probably try going down that path again
in the near future. It's a class of warnings that have the potential
to uncover real bugs (even if we don't currently have any) and it
would be a nice one to be able to enable by default in the Makefile.
I agree with you though that the "expression always false|true due to
unsigned" type of warnings are usually bogus - although there have
actually been real bugs hiding behind some of those warnings in the
past. But, I'll make sure to only submit patches for that type of
warnings in the future if I can prove that the warning actually
uncovered a real bug.
--
Jesper Juhl [email blocked]
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please http://www.expita.com/nomime.html
From: Linus Torvalds [email blocked]
Subject: Re: [PATCH] Don't compare unsigned variable for <0 in sys_prctl()
Date: Tue, 28 Nov 2006 16:13:05 -0800 (PST)
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
I agree!
Usually I'm a bigger champion of GCC's warnings, but I have to agree the unsigned warning issue Linus mentions IS annoying. It's entirely idiomatic to write "if (foo < FOOMIN || foo > FOOMAX) { .... }", and yet if FOOMIN == 0 and foo's unsigned, I get a warning. Bollocks.
After all, this is the kind of thing you might encapsulate in a macro. At least, I've done such things in the past as part of my debug code, a'la assert. For example:
#ifndef NDEBUG #define RCHECK(x,min,max) do { \ if ((x) < min || (x) > max) { /*log it*/ }\ } while(0) #endif /* ... */ RCHECK(foo, FOOMIN, FOOMAX);This is especially useful in an embedded context where the logging mechanism is something other than printf(), and just pulling down the system for a transient error is undesirable. Perfectly legit code.
Now as for the local-shadowing-global: Some of that is the C library's fault for polluting the name space with such generic names like "index." That one's just annoying. I blame BSD for that one specifically. ANSI confined itself to strXXX, which seems far more sensible. This particular whine could be addressed by a simple rule, though: Don't warn if the shadowed extern is a function.
The nested macro expansion is dicier. I can't think of a good answer there. GCC would have to track which locals came from macro expansion and which came from the program itself, and failing to warn on all scalar-to-scalar shadowing in nested blocks could hide very real errors. Where do you draw the line?
Not to be a C nazi, but that
Not to be a C nazi, but that macro has a small danger. Specifically, if someone passes a function call as the first argument then that call will happen twice because of how the macro works. Not great practice, I know, but people do it. It's a lot like the ctype.h mistakes from the earliest linux kernels.
Personally, I've seen too many bad things done with macros, so I generally prefer function calls. Even given call overhead I find them more predictable and generally simpler, and if the compiler inlines them that call overhead goes away.
No more dangerous than assert()
Note that if the first argument to RCHECK() has side effects, it's as broken as an argument to assert() that has side effects. Part of the reason I (and many people) capitalize macros is to point out that they're NOT functions, and therefore the semantics can be different. (Sometimes, quite different.)
That's really just C programming 101. If you haven't been schooled in the difference between macro invocation and function invocation, and the fact that many macros will expand their arguments more than once (and therefore the arguments should not have side effects), then perhaps you should stick to Pascal or maybe Java.
take a deep breath
Oh, I'm quite familiar with C programming (only been doing it for around 10 years, but that's given me a pretty good sampling) including the difference between macros and functions, but that doesn't save me from inheriting or using other people's code that isn't top form. As for macros, well, I have friends that go overboard and say that macros should never be used, mainly due to horrible abuse they suffered at previous jobs. One manager used macros with gotos to other macros *shudder*. Still, I do hate walking through in a debugger and not being able to trace into something because it's a macro.
In recent years I've come to dislike C simply because I got tired of reinventing the wheel, needing external libraries to do interesting things (think basic data structs, regular expressions, etc.), and because I've had to make things run between windows and linux. Currently I've lashed onto Python when I can get away with using it, but still have to write C and C++ fairly regularly. Personally hate Java, and don't even remember Pascal syntax anymore.
One final note, your second paragraph was a bit more acidic than it needed to be. I just pointed out a flaw that I've actually seen less experienced people run into. Over time I've become convinced that anything you can do to reduce the likelihood of someone else misusing your code (and often bitching to you about it) was a good thing, because I don't get to choose the skill level of the people adding onto or using my code.
*chuckle*
Sorry if that second paragraph came across the wrong way. I agree macros can be (and often are) overused and abused. I *tend* to use them for one-liner types of things. They are certainly something that should be used judiciously to be sure. Macros that GOTO other macros? Shoot the bastard already! :-)
In an ideal world, you'd combine conditional compilation with C++'s template mechanism to get something that's more likely to be type-sane etc. etc. since expansion happens at a level well above the token level, and eliminate the preprocessor. Then again, you'd lose the token-pasting feature that's every case-n-paster's dream.
I've personally been programming C about 14 years, and Pascal before that. And yes, in C you do often need to reinvent the wheel. As others have said, it's a portable assembly language. I would go further and say it's a portable assembly language with really bad pointer semantics.
I still hold that you can't consider yourself a competent C programmer unless you've wrapped your head around macros vs. functions and the differences in semantics. Proper idioms go a long way to making that a minor deal, by indicating in the style what semantics to expect. I'm not going to say "don't use the whole language because some people abuse it." That said, I'm rather skeptical about some of C++'s operator overloading habits...
Eh...
Try Modern Pascal - NOT STANDARD
All this talk about Standard Pascal. Get with the times! Standard Pascal is dead and is no longer improved.
Modern Pascal however, allows pre processor macros (but they are discouraged) and offers separate compilation of units, and allows you to do nasty pointer work that C allows, and include files are available, and DEFINES are available, etc. etc.
You folks should check out modern pascal compilers like Freepascal (not GNU pascal) because all this talk about Pascal is rubbish - unless you are referencing Standard Pascal, which is 100 percent dead now and no longer improved for real world programming.
I've programmed in Modern C and Modern Pascal and I find modern Pascal as powerful as C++, but cleaner since C++ gets a little arcane at times. Standard Pascal is no where near as powerful as C, but modern pascal is.
By the way - I shoot myself in the foot all the time with Modern Pascal because I turn range checking off and pointer checking off.
I've only used Standard Pascal once
The only Standard Pascal compiler I ever used was a slow abortion of an environment on an Apple ///. (Or maybe it was an Apple ][e.) It SUCKED.
I did all my real Pascal programming under Turbo Pascal, using versions 3.0 up through 7.0 and a little bit of Borland Pascal. Sure, it may not be as whizzy as whatever Pascal variant you're programming now, but Turbo Pascal was certainly not standard, especially with inline asm thrown around. :-)
B is not C and Pascal is not 1970
Yes you are quite right, turbopascal was not standard, and it was a 1980's version of a more modern pascal. Freepascal is a 1990's/2000's version of Pascal, and is even better than turbo pascal - much better.
I didn't mean to directly respond to your post - it was Linus and the other person who were flaming pascal for being so "strict" that you can't even do useful things - but they are out of touch with the times - linus should have stated "standard pascal from the 1970's" and not "pascal" in general. Saying Pascal in general implies "all Pascal's including 2006 freepascal". So if I state that "C sucks" am I talking about the original B language which C was partly based on, or am I talking about 1980's or 1990's C. Well, C doesn't suck - and neither does Pascal. Because things improve. C is not B, and Pascal is not 1970 Pascal. Get with the times, Linus.
So "modern PASCAL" is a combi
So "modern PASCAL" is a combination of the type-mess that is C(++) and the baroque Syntax of PASCAL. The worst of both worlds. So far I'm not convinced.
Nice trolling. There's _no_
Nice trolling.
There's _no_ type mess. FPC and Delphi have (had for years) native, CPU-word max size, reference-counted, COW, String type. Native boolean type. Boolean, char, int and pointer are _never_ _implicitly_ interchangeable.
Beyond that, it has native, powerful and clean RTTI. First class exception handling. Intuitive and unobtrusive namespace handling. Efficient "header" handling. Compiles tens of times faster than GCC or MSVC.
And can please tell what's baroque about Pascal syntax so I don't confuse for one of those sillies who cry fool on Pascal verbosiness, like there was a single IDE which don't have code templates to write 'begin..end' for you, or like we were still in the 70s and '&&' is any clearer or friendlier than 'and'...
And can please tell what's ba
And can please tell what's baroque about Pascal syntax so I don't confuse for one of those sillies who cry fool on Pascal verbosiness, like there was a single IDE which don't have code templates to write 'begin..end' for you, or like we were still in the 70s and '&&' is any clearer or friendlier than 'and'...
And please tell me what's so bad about the fact that I weigh 450 pounds. It's not like powered exoskeletons haven't been invented! And I can use special tools to put my shoes on, despite the fact that I'm so unwieldy!
ELSE is verbose and so is
ELSE is verbose and so is PRINTF. So why are the words ELSE and PRINTF used in the C language? If they were smart (the C language designers) they would have chosen ELS, and JOT or OUT instead of ELSE and PRINT, COUT, or PRINTF. So why do you bother using C or C++ if it is so verbose? Generally because it usually doesn't matter all that much that you have to type out PRINT or ELSE.
Also, a simple FOR loop is actually more VERBOSE in C. It looks terse but if you count the characters in C there are actually more characters to set up than in Pascal. True you can do more complex FOR loops but generally people use WHILE loops when the for loop becomes more complex. The same issue haunts JAVA. Java's FOR loop is very verbose with a lot of symbols - which fools programmers into thinking it is terse.
Count the characters..
FOR blah to blah DO
something;
Now write that in C and see how many SHIFT KEYS you have to pain yourself with... and count the total characters. Sometimes it is not just the verbosity of a language, but the amount of shift keys you have to stretch for. Don't the amount of shift keys in this piss you off:
for (blah; blah; i++){}
And that isn't even finished yet.. I didn't even complete it.. and it is already pissing me off. So if you are going to complain about verbosity, start complaining about SHIFT KEY annoyance too. And don't tell me you can reconfigure your keyboard, because you can't - that ruins the easy square bracket access since they are on the same key as a curly. Plus reconfiguring the keyboard ruins you when you have to travel or you use someone else's computer, or you have several computers to maintain, or you are logging into a remote PC.
Almost all for loops are simple for loops and we rarely ever need to use the complexity available in the C loop.. and when we do need to have a more complex for loop simple use a WHILE loop.
However, that being said, I still have a hot spot for certain terseness (not perl terseness) when it is just as readable.. which is what will be put into the Qompute language. PEN or JOT is just as readable as WRITE or PRINT, and ELS is just as readable as ELSE. So why not utilize those terse but readable shortforms. COUT however is not readable because COUT can be pronounced and it is hard to tell what COUT really means just by looking at it. COUT is also more verbose than OUT, JOT, or PEN. True after one learns C it is obvious what COUT means - but they could have just chosen OUT instead. OUT would have conflicted with OUT macro that is used for paramaters maybe (i.e. COM programming) so still a better choice is JOT.
I guess I disagree with Linus
I guess I disagree with Linus here.
I can't see how the sign range is different from the size range or the type range. IOW: you are comparing different types, the warning is deserved.
And the useless comparison should be removed.
Or else, why the variable is declared unsigned after all? Should all users of it write code as if somebody could change its declaration to signed at any time?
At minimum the warning can help detect somebody using the wrong variable...
I agree with you. At one ext
I agree with you. At one extreme, the compiler could just remove the unnecessary comparison since it's never going to resolve to "true". But it tends to make people paranoid when compilers do things like that, so GCC's compromise was to just throw a warning. This lets the user know about code he might personally want to optimize out, but that the compiler isn't going to touch.
W/ regard to the macro issue raised above- if you know that it might generate a warning, encase the body of the macro in a _pragma that disables that particular warning.
Pascal
Really what is the matter with Standard Pascal talk? Pascal is an obsolete language...
Gee, assuming a language can
Gee, assuming a language can or should die, you are totally correct. But hey, at least people are still happy with Pascal and making things just as good as you without inline assembler. (Or did you forget that pascal has always compiled to cleaner assembler?) And Pascal is easier to learn, oh and it is easier to read, and it is still applicable to current markets. You confuse Standard Pascal, the old Pascal, with the current Pascal (Modern Pascal) which is very competitive.
You say Pascal is obsolete, but you haven't done a damned thing to prove it. People like those who design Samurize work to prove otherwise.
Safety
I suppose I am kicking a dead horse here, but the unsigned warning is actually a very, very bad warning. Say I see that warning and "fix" the offending code. Now say that I later change the type of the variable to signed or that I raise the minimum to a higher number. I am now officially screwed over.