[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [Bug 1655708] [NEW] target/ppc/int_helper.c:2806: stran
From: |
joserz |
Subject: |
Re: [Qemu-devel] [Bug 1655708] [NEW] target/ppc/int_helper.c:2806: strange expression ? |
Date: |
Wed, 11 Jan 2017 16:01:42 -0200 |
User-agent: |
Mutt/1.5.24 (2015-08-30) |
On Wed, Jan 11, 2017 at 05:12:38PM -0000, Eric Blake wrote:
> On 01/11/2017 10:41 AM, dcb wrote:
> > Public bug reported:
> >
> > target/ppc/int_helper.c:2806:25: warning: ‘*’ in boolean context,
> > suggest ‘&&’ instead [-Wint-in-bool-context]
> >
> > Source code is
> >
> > zone_digit = (i * 2) ? b->u8[BCD_DIG_BYTE(i * 2)] >> 4 :
> > zone_lead;
>
> Also, looking at BCD_DIG_BYTE():
>
> #if defined(HOST_WORDS_BIGENDIAN)
> #define BCD_DIG_BYTE(n) (15 - (n/2))
> #else
> #define BCD_DIG_BYTE(n) (n/2)
> #endif
>
> Oops. n is under-parenthesized, and will cause invalid expansions for
> some expressions. Let's fix that as well.
>
Hello guys!!
Do you mean something like:
BCD_DIG_BYTE(n) (15 - ((n)/2)) and BCD_DIG_BYTE(n) ((n)/2) ?
to avoid issues like BCD_DIG_BYTE(i + 3) that will expand to (i + 3/2)
instead of ((i + 3)/2), right?
>
> > so I think the compiler warning is for the i * 2 lhs of the ?.
>
> Yes - the compiler is complaining that 'i * 2' can only be non-zero if
> 'i' was non-zero (given that the code occurs in a loop for i between 0
> and 16), so it is just as easy to write 'i ? ...' instead of the weirder
> '(i * 2) ? ...'.
>
Yes, Eric is correct it's meant to be "(i) ? .. : .." because the sign
is located at position 0. I'm improving it. I'll send a patch for them
today.
Thanks!