All of lore.kernel.org
 help / color / mirror / Atom feed
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Rasmus Villemoes <rv@rasmusvillemoes.dk>,
	Sparse Mailing-list <linux-sparse@vger.kernel.org>,
	Christopher Li <sparse@chrisli.org>
Subject: Re: [PATCH] sparse: Make struct token::special signed
Date: Wed, 24 Dec 2014 06:01:30 +0100	[thread overview]
Message-ID: <1419397290.27103.72.camel@kernel.crashing.org> (raw)
In-Reply-To: <CA+55aFwW30VbRy7uiBZn-gmnYi7B7jr8g=rvR-_uhYxV3U6_Vw@mail.gmail.com>

On Tue, 2014-12-23 at 11:01 -0800, Linus Torvalds wrote:
> On Tue, Dec 23, 2014 at 5:16 AM, Rasmus Villemoes <rv@rasmusvillemoes.dk> wrote:
> > There doesn't seem to be any reason for the special member of struct
> > token to be unsigned; AFAICT it is only ever being directly compared
> > to explicit characters and the SPECIAL_* enum constants using ==, !=
> > and in a switch statement. Making it plain int avoids an annoying
> > warning from match_op in token.h when compiling with -Wsign-compare.
> 
> Please don't use -Wsign-cpmpare to make decisions about code.
> 
> "unsigned" is generally the much preferred type if there are no
> reasons for it to be signed. And -Wsign-compare on its own is not a
> reason, since it gives insane warnings for good code.
> 
> -Wsign-compare is basically a "you can walk through the warnings and
> see if any of them are actually valid" thing. It's not worth it in any
> other form. Trying to be sign-compare clean will result in actively
> *worse* code in some circumstances (ie pointless casts etc etc).

Additionally "compare with characters" trips another flag, chars are
unsigned by default on some archs :)

Cheers
Ben.

>                      Linus
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



  reply	other threads:[~2014-12-24  5:01 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-23 13:16 [PATCH] sparse: Make struct token::special signed Rasmus Villemoes
2014-12-23 19:01 ` Linus Torvalds
2014-12-24  5:01   ` Benjamin Herrenschmidt [this message]
2014-12-24 11:07     ` Rasmus Villemoes

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=1419397290.27103.72.camel@kernel.crashing.org \
    --to=benh@kernel.crashing.org \
    --cc=linux-sparse@vger.kernel.org \
    --cc=rv@rasmusvillemoes.dk \
    --cc=sparse@chrisli.org \
    --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.