Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Daniel Jacobowitz <drow@false.org>
To: Mark Kettenis <mark.kettenis@xs4all.nl>
Cc: gdb-patches@sourceware.org
Subject: Re: RFC: Warning fixes
Date: Thu, 28 Dec 2006 23:09:00 -0000	[thread overview]
Message-ID: <20061228230925.GA23775@nevyn.them.org> (raw)
In-Reply-To: <200612282256.kBSMu65R022410@brahms.sibelius.xs4all.nl>

On Thu, Dec 28, 2006 at 11:56:06PM +0100, Mark Kettenis wrote:
> > Date: Thu, 28 Dec 2006 14:58:28 -0500
> > From: Daniel Jacobowitz <drow@false.org>
> > 
> > Any comments on these, or shall I commit them?  Regardless of the configure
> > patch, we might as well fix the bugs.
> 
> Some of these are really scary.  Can you check for regressions on i386 Linux?

I haven't yet, but definitely will before checking them in.

> > -  if ((sig >= TARGET_SIGNAL_FIRST) && (sig <= TARGET_SIGNAL_LAST))
> > +  if (sig <= TARGET_SIGNAL_LAST)

> ISO C clearly states that enumeration constants have type 'int', so
> sig could be negative, and we defenitely want to catch that case.  If
> GCC thinks it knows that (sig >= TARGET_SIGNAL_FIRST), I think it is
> being too smart for its own good.

If you want to catch that case, then it's a good thing you had this
warning here to look after you, because the old code might not do it
either :-)  Though it always works out, after promotion, because of
the second test.

This happens because TARGET_SIGNAL_FIRST is an "int", but "sig" is
an enum.  The choice of compatible integer type for an enum is
implementation defined, and GCC documents:

   * `The integer type compatible with each enumerated type (C90
     6.5.2.2, C99 6.7.2.2).'

     Normally, the type is `unsigned int' if there are no negative
     values in the enumeration, otherwise `int'.  If `-fshort-enums' is
     specified, then if there are negative values it is the first of
     `signed char', `short' and `int' that can represent all the
     values, otherwise it is the first of `unsigned char', `unsigned
     short' and `unsigned int' that can represent all the values.

There's nothing negative in the enum, ergo GCC chooses unsigned int.
And therefore TARGET_SIGNAL_FIRST is promoted to unsigned int.

The warning is a bit annoying, though.  We can't portably tell whether
the type of sig will be signed or unsigned.  Perhaps we should force
sig to be an int before bounds checking, instead, and I should file a
GCC bug report.  How's that sound to you?

-- 
Daniel Jacobowitz
CodeSourcery


  parent reply	other threads:[~2006-12-28 23:09 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-12-28 19:58 Daniel Jacobowitz
2006-12-28 22:56 ` Mark Kettenis
2006-12-28 23:08   ` Andreas Schwab
2006-12-28 23:09   ` Daniel Jacobowitz [this message]
2006-12-29  3:48     ` Daniel Jacobowitz
2007-01-03 19:01       ` Daniel Jacobowitz
2006-12-29 12:15     ` Eli Zaretskii
2007-01-03 18:47       ` Daniel Jacobowitz

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=20061228230925.GA23775@nevyn.them.org \
    --to=drow@false.org \
    --cc=gdb-patches@sourceware.org \
    --cc=mark.kettenis@xs4all.nl \
    /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