From: Linus Torvalds <torvalds@linux-foundation.org>
To: Sergei Organov <osv@javad.com>
Cc: "Pekka Enberg" <penberg@cs.helsinki.fi>,
"J.A. MagallÃÃÃón" <jamagallon@ono.com>,
"Jan Engelhardt" <jengelh@linux01.gwdg.de>,
"Jeff Garzik" <jeff@garzik.org>,
"Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
"Andrew Morton" <akpm@linux-foundation.org>
Subject: Re: somebody dropped a (warning) bomb
Date: Thu, 15 Feb 2007 07:57:05 -0800 (PST) [thread overview]
Message-ID: <Pine.LNX.4.64.0702150736360.20368@woody.linux-foundation.org> (raw)
In-Reply-To: <87hctnlfqz.fsf@javad.com>
On Thu, 15 Feb 2007, Sergei Organov wrote:
>
> Yes, there is. There are at least 2 drawbacks. Minor one is
> initialization eating cycles. Major one is that if later I change the
> code and there will appear a pass that by mistake uses those fake value,
> I won't get the warning anymore. The latter drawback is somewhat similar
> to the drawbacks of explicit type casts.
I actually agree - it would clearly be *nicer* if gcc was accurate with
the warning. I'm just saying that usually the patch to shut gcc up is
quite benign.
But yes, it basically disabled the warning, and just *maybe* the warning
was valid, and it disabled it by initializing something to the wrong
value.
That's why warnings with false positives are often much worse than not
having the warning at all: you start dismissing them, and not even
bothering to fix them "properly". That's especially true if there _isn't_
a proper fix.
I personally much prefer a compiler that doesn't warn a lot, but *when* it
warns, it's 100% on the money. I think that compilers that warn about
things that "may" be broken are bogus. It needs to be iron-clad, with no
question about whether the warning is appropriate!
Which is, of course, why we then shut up some gcc warnings, and which gets
us back to the start of the discussion.. ;)
> [I'd, personally, try to do code reorganization instead so that it
> becomes obvious for the compiler that the variable can't be used
> uninitialized. Quite often it makes the code better, at least it was my
> experience so far.]
It's true that it sometimes works that way. Not always, though.
The gcc "uninitialized variable" thing is *usually* a good sign that the
code wasn't straightforward for the compiler to follow, and if the
compiler cannot follow it, then probably neither can most programmers. So
together with the fact that it's not at least _syntactically_ ugly to shut
up, it's not a warning I personally object to most of the time (unlike,
say, the pointer-sign one ;)
> [Did I already say that I think it's wrong warning to be given in this
> particular case, as the problem with the code is not in signedness?]
>
> 1. Don't try to hide the warning, at least not immediately, -- consider
> fixing the code first. Ah, sorry, you've already decided for yourself
> that the warning is idiotic, so there is no reason to try to...
This is actually something we've tried.
The problem with that approach is that once you have tons of warnings that
are "expected", suddenly *all* warnings just become noise. Not just the
bogus one. It really *is* a matter of: warnings should either be 100%
solid, or they should not be there at all.
And this is not just about compiler warnings. We've added some warnings of
our own (well, with compiler help, of course): things like the
"deprecated" warnings etc. I have often ended up shutting them up, and one
reason for that is that yes, I have literally added bugs to the kernel
that the compiler really *did* warn about, but because there were so many
other pointless warnings, I didn't notice!
I didn't make that up. I forget what the bug was, but it literally wasn't
more than a couple of months ago that I passed in the wrong type to a
function, and the compiler warned about it in big letters. It was just
totally hidden by all the other warning crap.
> 2. If you add a cast, it's not in contrast, it's rather similar to fake
> initialization above as they have somewhat similar drawback.
I agree that it's "unnecessary code", and in many ways exactly the same
thing. I just happen to believe that casts tend to be a lot more dangerous
than extraneous initializations. Casts have this nasty tendency of hiding
*other* problems too (ie you later change the thing you cast completely,
and now the compiler doesn't warn about a *real* bug, because the cast
will just silently force it to a wrong type).
And yeah, that may well be a personal hangup of mine. A lot of people
think casts in C are normal. Me, I actually consider C to be a type-safe
language (!) but it's only type-safe if you are careful, and avoiding
casts is one of the rules.
Others claim that C isn't type-safe at all, and I think it comes from
them having been involved in projects where people didn't have the same
"casts are good, but only when you use them really really carefully".
> > But if you have
> >
> > unsigned char *mystring;
> >
> > len = strlen(mystring);
> >
> > then please tell me how to fix that warning without making the code
> > *worse* from a type-safety standpoint? You CANNOT. You'd have to cast it
> > explicitly (or assing it through a "void *" variable), both of which
> > actually MAKE THE TYPE-CHECKING PROBLEM WORSE!
>
> Because instead of trying to fix the code, you insist on hiding the
> warning.
No. I'm saying that I have *reasons* that I need my string to be unsigned.
That makes your first "fix" totally bogus:
> There are at least two actual fixes:
>
> 1. Do 'char *mystring' instead, as otherwise compiler can't figure out
> that you are going to use 'mystring' to point to zero-terminated
> array of characters.
And the second fix is a fix, but it is, again, worse than the warning:
> 2. Use "len = ustrlen(mystring)" if you indeed need something like C
> strings
Sure, I can do that, but the upside is what?
Getting rid of a warning that was bogus to begin with?
Linus
next prev parent reply other threads:[~2007-02-15 15:57 UTC|newest]
Thread overview: 68+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-02-08 15:00 somebody dropped a (warning) bomb Jeff Garzik
2007-02-08 16:33 ` Linus Torvalds
2007-02-08 18:42 ` Jan Engelhardt
2007-02-08 19:53 ` Linus Torvalds
2007-02-08 21:10 ` Jan Engelhardt
2007-02-08 21:37 ` Linus Torvalds
2007-02-08 23:12 ` David Rientjes
2007-02-08 23:37 ` Linus Torvalds
2007-02-09 0:24 ` David Rientjes
2007-02-09 0:42 ` Linus Torvalds
2007-02-09 0:59 ` Linus Torvalds
2007-02-09 0:59 ` David Rientjes
2007-02-09 1:11 ` Linus Torvalds
2007-02-09 1:18 ` David Rientjes
2007-02-09 15:38 ` Linus Torvalds
2007-02-09 3:27 ` D. Hazelton
2007-02-09 19:54 ` Pete Zaitcev
2007-02-09 12:34 ` Jan Engelhardt
2007-02-09 13:16 ` linux-os (Dick Johnson)
2007-02-09 17:45 ` Jan Engelhardt
2007-02-09 20:29 ` linux-os (Dick Johnson)
2007-02-09 22:05 ` Jan Engelhardt
2007-02-09 22:58 ` Martin Mares
2007-02-12 18:50 ` linux-os (Dick Johnson)
2007-02-13 15:14 ` Dick Streefland
2007-02-08 21:13 ` J.A. Magallón
2007-02-08 21:42 ` Linus Torvalds
2007-02-08 22:03 ` Linus Torvalds
2007-02-08 22:19 ` Willy Tarreau
2007-02-09 0:03 ` J.A. Magallón
2007-02-09 0:22 ` Linus Torvalds
2007-02-09 12:38 ` Sergei Organov
2007-02-09 15:58 ` Linus Torvalds
2007-02-12 11:12 ` Sergei Organov
2007-02-12 16:26 ` Linus Torvalds
2007-02-13 18:06 ` Sergei Organov
2007-02-13 18:26 ` Pekka Enberg
2007-02-13 19:14 ` Sergei Organov
2007-02-13 19:43 ` Pekka Enberg
2007-02-13 20:29 ` Sergei Organov
2007-02-13 21:31 ` Jeff Garzik
2007-02-13 23:21 ` Linus Torvalds
2007-02-15 13:20 ` Sergei Organov
2007-02-15 15:57 ` Linus Torvalds [this message]
2007-02-15 18:53 ` Sergei Organov
2007-02-15 19:02 ` Linus Torvalds
2007-02-15 20:23 ` me, not " Oleg Verych
2007-02-16 4:26 ` Rene Herman
2007-02-19 11:58 ` Sergei Organov
2007-02-19 13:58 ` Sergei Organov
2007-02-15 22:32 ` Lennart Sorensen
2007-02-13 19:25 ` Linus Torvalds
2007-02-13 19:59 ` Sergei Organov
2007-02-13 20:24 ` Linus Torvalds
2007-02-15 15:15 ` Sergei Organov
2007-02-13 21:13 ` Rob Landley
2007-02-13 22:21 ` Olivier Galibert
2007-02-14 12:52 ` Sergei Organov
2007-02-15 20:06 ` Sergei Organov
2007-02-09 15:10 ` Sergei Organov
2007-02-08 16:35 ` Kumar Gala
[not found] <7Mj5f-3oz-21@gated-at.bofh.it>
[not found] ` <7MktH-5EW-35@gated-at.bofh.it>
[not found] ` <7Mmvy-vj-17@gated-at.bofh.it>
[not found] ` <7MnBC-2fk-13@gated-at.bofh.it>
[not found] ` <7MoQx-4p8-11@gated-at.bofh.it>
[not found] ` <7MpjE-50z-7@gated-at.bofh.it>
[not found] ` <7MpCS-5Fe-9@gated-at.bofh.it>
[not found] ` <7MDd7-17w-1@gated-at.bofh.it>
[not found] ` <7MGkB-62k-31@gated-at.bofh.it>
[not found] ` <7NHoe-2Mb-37@gated-at.bofh.it>
[not found] ` <7NMe9-1ZN-7@gated-at.bofh.it>
[not found] ` <7Oagl-6bO-1@gated-at.bofh.it>
[not found] ` <7ObvW-89N-23@gated-at.bofh.it>
[not found] ` <7Oc8t-NS-1@gated-at.bofh.it>
2007-02-15 20:08 ` Bodo Eggert
2007-02-16 11:21 ` Sergei Organov
2007-02-16 14:51 ` Bodo Eggert
2007-02-19 11:56 ` Sergei Organov
2007-02-16 12:46 ` Sergei Organov
2007-02-16 17:40 ` Bodo Eggert
2007-02-19 12:17 ` Sergei Organov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Pine.LNX.4.64.0702150736360.20368@woody.linux-foundation.org \
--to=torvalds@linux-foundation.org \
--cc=akpm@linux-foundation.org \
--cc=jamagallon@ono.com \
--cc=jeff@garzik.org \
--cc=jengelh@linux01.gwdg.de \
--cc=linux-kernel@vger.kernel.org \
--cc=osv@javad.com \
--cc=penberg@cs.helsinki.fi \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).