Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Chen Gang <gang.chen.5i5j@gmail.com>
To: Pedro Alves <palves@redhat.com>,  Joel Brobecker <brobecker@adacore.com>
Cc: Mark Kettenis <mark.kettenis@xs4all.nl>,
	amodra@gmail.com,  gbenson@redhat.com, michael.sturm@intel.com,
	walfred.tedeschi@intel.com,  binutils@sourceware.org,
	gdb-patches@sourceware.org
Subject: Re: [PATCH] gdb/i387-tdep.c: Avoid warning for "-Werror=strict-overflow"
Date: Mon, 06 Oct 2014 13:29:00 -0000	[thread overview]
Message-ID: <54329A9A.9090303@gmail.com> (raw)
In-Reply-To: <543255B6.7060509@redhat.com>

On 10/6/14 16:41, Pedro Alves wrote:
> On 10/04/2014 06:18 AM, Chen Gang wrote:
>>
>> OK, thanks. It is really one way, it is a little better than my original
>> way. But for me, it is still not a good idea: it introduces a new macro
>> and a new variable for each area (originally, it is only one statement).
> 
> I see no problem with adding the new macro.  We already have a ton
> of similar macros, see i386-tdep.h and i387-tdep.h.  Looks
> like the existing I387_NUM_REGS is what we'd need here?
> 
> BTC, OOC, did you try Joel's idea with the local variable?
> In case Mark prefers that, it'd be good to know whether it works.
> I can't seem to get my gcc to emit that warning.
> 
> Combining both ideas, for clarity, we end up with something
> like:
> 
>  int end;
> 
>  end = I387_ST0_REGNUM (tdep) + I387_NUM_REGS;
>  for (i = I387_ST0_REGNUM (tdep); i < end; i++)
> 
>  ...
> 
>  end = I387_XMM0_REGNUM (tdep) + I387_NUM_XMM_REGS (tdep);
>  for (i = I387_XMM0_REGNUM (tdep); i < end; i++)
> 
> 
> That's way clearer to me than the existing:
> 

That's way not quite bad to me than the existing:

 - It is easier understanding, although a little complex than origin.

 - For compiler, 'end' is simple enough to be sure to be optimized.

 - And I guess, compiler will understand, and will not worry about it.


>  for (i = I387_ST0_REGNUM (tdep); i < I387_XMM0_REGNUM (tdep); i++)
> ...
>  for (i = I387_XMM0_REGNUM (tdep); i < I387_MXCSR_REGNUM (tdep); i++)
> 
> anyway, which assumes the reader knows register numbers are
> ordered like st -> xmm -> mxcrsr.
> 
> If this works, I think it's my preference.
> 

OK, thanks, at least, what you said is acceptable to me. If no any
additional reply within this week (within 2014-10-12), I shall send
patch v2 for it.

>> For me, "-Werror" need always be optional, but not mandatory.
> 
> It's mandatory only on development builds.  -Werror is not on by
> default on released GDBs.
> 

OK, thanks.


Thanks.
-- 
Chen Gang

Open, share, and attitude like air, water, and life which God blessed


  reply	other threads:[~2014-10-06 13:29 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-10-03 15:24 Chen Gang
2014-10-03 15:46 ` Mark Kettenis
2014-10-03 16:02   ` Chen Gang
2014-10-03 16:44     ` Joel Brobecker
2014-10-03 18:47       ` Pedro Alves
2014-10-04  5:12         ` Chen Gang
2014-10-06  8:41           ` Pedro Alves
2014-10-06 13:29             ` Chen Gang [this message]
2014-10-10 11:22               ` Chen Gang
2014-10-09 10:06         ` Walfred Tedeschi
2014-10-09 11:20           ` Pedro Alves

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=54329A9A.9090303@gmail.com \
    --to=gang.chen.5i5j@gmail.com \
    --cc=amodra@gmail.com \
    --cc=binutils@sourceware.org \
    --cc=brobecker@adacore.com \
    --cc=gbenson@redhat.com \
    --cc=gdb-patches@sourceware.org \
    --cc=mark.kettenis@xs4all.nl \
    --cc=michael.sturm@intel.com \
    --cc=palves@redhat.com \
    --cc=walfred.tedeschi@intel.com \
    /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