All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@kernel.org>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Chris Metcalf <cmetcalf@ezchip.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Borislav Petkov <bp@alien8.de>,
	open list <linux-kernel@vger.kernel.org>,
	"H. Peter Anvin" <hpa@zytor.com>
Subject: Re: [PATCH] string: Improve the generic strlcpy() implementation
Date: Mon, 5 Oct 2015 16:07:39 +0200	[thread overview]
Message-ID: <20151005140739.GA7478@gmail.com> (raw)
In-Reply-To: <CA+55aFx2McOeEiB7fJ-BV=vBsH=i2cC-qW8_EBEnScfQhugD_w@mail.gmail.com>


* Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Oct 5, 2015 14:15, "Ingo Molnar" <mingo@kernel.org> wrote:
> >
> > Hm, so GCC (v4.9.2) will only warn about this bug if -Wtype-limits is enabled 
> > explicitly:
> 
> Some of the warnings are really nasty, and cause people to write worse code.
> 
> For example, this is inherently good code:
> 
>     if (x < 0 || x > MAXLEN)
>         return -EINVAL;
> 
> and a compiler that warns about that is pure and utter crap. Obviously. Agreed?
> 
> Now, imagine that "x" here is some random type. Maybe it's s "char" and you 
> don't even know the sign. Maybe it's "loff_t". Maybe it's "size_t", or whatever.
> 
> Note how that test is correct *regardless* of the sign of the type. A compiler 
> that warns about the "x < 0" part just because x happens to be unsigned is a bad 
> bad compiler, and makes people remove that check, even though it's good for 
> readability, and good for robustness wrt changing the type.

Yeah, that's true.

> We really do have types where sightedness depends on architecture or
> possibly configuration options. "char" is the obvious example, but the type
> limit can matter too: on some architectures you might have a type that is
> 16 bits, on another it might be 32 bits. Do you really think that
> 
>      if (x > 65535)
>           return -E2BIG;
> 
> should have some #ifdef __xyz__ around it just because the compiler warns
> if the type happens to be 16 bits wide?
> 
> So type limit warnings break not things than they fix.

Yeah, too bad.

> These things come up in macros too (think range checking etc).
> 
> In other words, that warning really isn't good if it's done mindlessly. And I've 
> never seen a compiler that did it sanely and trying to take context into 
> account.
> 
> So no. Don't enable that broken warning. We have had it on, and it caused people 
> to send in patches for warnings that made the code actively worse.

Okay. Please disregard my other mail.

Thanks,

	Ingo

  parent reply	other threads:[~2015-10-05 14:07 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-10 19:43 [GIT PULL] strscpy string copy function Chris Metcalf
2015-10-04 15:55 ` Linus Torvalds
2015-10-05 11:27   ` [PATCH] string: Improve the generic strlcpy() implementation Ingo Molnar
2015-10-05 11:53     ` Ingo Molnar
2015-10-05 13:15       ` Ingo Molnar
2015-10-05 14:04         ` Ingo Molnar
     [not found]         ` <CA+55aFx2McOeEiB7fJ-BV=vBsH=i2cC-qW8_EBEnScfQhugD_w@mail.gmail.com>
2015-10-05 14:07           ` Ingo Molnar [this message]
2015-10-05 14:33           ` Ingo Molnar
2015-10-05 15:32             ` Linus Torvalds
2015-10-05 16:03               ` Ingo Molnar
2015-10-05 12:28     ` Linus Torvalds
2015-10-05 13:10       ` Ingo Molnar
2015-10-05 22:28     ` Rasmus Villemoes
2015-10-06  7:54       ` Ingo Molnar
2015-10-06  8:03       ` Ingo Molnar
2015-10-06 22:00         ` Rasmus Villemoes
2015-10-07  7:18           ` Ingo Molnar
2015-10-07  9:04             ` Rasmus Villemoes
2015-10-07  9:22               ` Linus Torvalds
2015-10-08  8:48                 ` Ingo Molnar
2015-10-09  8:10                   ` Rasmus Villemoes
2015-10-09  9:10                     ` [RFC 0/3] eliminate potential race in string() (was: [PATCH] string: Improve the generic strlcpy() implementation) Rasmus Villemoes
2015-10-09  9:14                       ` [RFC 1/3] lib/vsprintf.c: pull out padding code from dentry_name() Rasmus Villemoes
2015-10-09  9:14                       ` [RFC 2/3] lib/vsprintf.c: move string() below widen_string() Rasmus Villemoes
2015-10-09  9:14                       ` [RFC 3/3] lib/vsprintf.c: eliminate potential race in string() Rasmus Villemoes
2015-10-10  7:47                       ` [RFC 0/3] eliminate potential race in string() (was: [PATCH] string: Improve the generic strlcpy() implementation) Ingo Molnar
2015-10-19 12:42     ` [PATCH] string: Improve the generic strlcpy() implementation Rasmus Villemoes
2015-10-19 16:24       ` Chris Metcalf
  -- strict thread matches above, loose matches on Subject: below --
2015-10-05 15:38 Alexey Dobriyan
2015-10-05 16:11 ` Ingo Molnar
2015-10-05 16:13   ` Ingo Molnar
     [not found] ` <CA+55aFyTVJfCt00gYJpiQW5kqPaRGJ93JmfRRni-73zCf5ivqg@mail.gmail.com>
2015-10-05 16:22   ` Ingo Molnar
2015-10-05 16:28     ` Ingo Molnar
2015-10-05 20:40     ` Linus Torvalds

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=20151005140739.GA7478@gmail.com \
    --to=mingo@kernel.org \
    --cc=a.p.zijlstra@chello.nl \
    --cc=bp@alien8.de \
    --cc=cmetcalf@ezchip.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.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.