All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Jesper Juhl" <jesper.juhl@gmail.com>
To: "Linus Torvalds" <torvalds@osdl.org>
Cc: linux-kernel@vger.kernel.org, "Andrew Morton" <akpm@osdl.org>,
	trivial@kernel.org
Subject: Re: [PATCH] Don't compare unsigned variable for <0 in sys_prctl()
Date: Wed, 29 Nov 2006 02:10:43 +0100	[thread overview]
Message-ID: <9a8748490611281710g78402fbeh8ff7fcc162dbcbca@mail.gmail.com> (raw)
In-Reply-To: <Pine.LNX.4.64.0611281600020.4244@woody.osdl.org>

On 29/11/06, Linus Torvalds <torvalds@osdl.org> wrote:
>
>
> 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.
>

I can't really say anything else at this point but, point conceded...

-- 
Jesper Juhl <jesper.juhl@gmail.com>
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

  reply	other threads:[~2006-11-29  1:10 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-11-28 22:17 [PATCH] Don't compare unsigned variable for <0 in sys_prctl() Jesper Juhl
2006-11-28 22:27 ` Linus Torvalds
2006-11-28 22:34   ` Jesper Juhl
2006-11-28 23:06     ` Linus Torvalds
2006-11-28 23:42       ` Jesper Juhl
2006-11-29  0:13         ` Linus Torvalds
2006-11-29  1:10           ` Jesper Juhl [this message]
2006-11-28 22:58 ` Friends dont let friends use GCC -W (was Re: [PATCH] Don't compare unsigned Valdis.Kletnieks

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=9a8748490611281710g78402fbeh8ff7fcc162dbcbca@mail.gmail.com \
    --to=jesper.juhl@gmail.com \
    --cc=akpm@osdl.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvalds@osdl.org \
    --cc=trivial@kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.