[Qemu-devel] [Bug 1655708] [NEW] target/ppc/int_helper.c:2806: strange expression ?

[Qemu-devel] [Bug 1655708] [NEW] target/ppc/int_helper.c:2806: strange expression ?

[dcb]

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;

Which I read as

       zone_digit = (i * 2) ? (b->u8[BCD_DIG_BYTE(i * 2)] >> 4) :
zone_lead;

so I think the compiler warning is for the i * 2 lhs of the ?.

I am not sure what to suggest as a bugfix.

** Affects: qemu
     Importance: Undecided
         Status: New

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1655708

Title:
  target/ppc/int_helper.c:2806: strange expression ?

Status in QEMU:
  New

Bug description:
  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;

  Which I read as

         zone_digit = (i * 2) ? (b->u8[BCD_DIG_BYTE(i * 2)] >> 4) :
  zone_lead;

  so I think the compiler warning is for the i * 2 lhs of the ?.

  I am not sure what to suggest as a bugfix.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1655708/+subscriptions

Re: [Qemu-devel] [Bug 1655708] [NEW] target/ppc/int_helper.c:2806: strange expression ?

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 989 bytes --]

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.


> 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) ? ...'.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [Bug 1655708] [NEW] target/ppc/int_helper.c:2806: strange expression ?

[Thomas Huth]

[-- Attachment #1: Type: text/plain, Size: 523 bytes --]

On 11.01.2017 18:12, Eric Blake wrote:
> On 01/11/2017 10:41 AM, dcb wrote:
>> 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) ? ...'.

... unless something like (i & 2) was meant instead?
Maybe Jose (who wrote that code) could comment on this?

 Thomas




[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [Qemu-devel] [Bug 1655708] [NEW] target/ppc/int_helper.c:2806: strange expression ?

[joserz]

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!

[Qemu-devel] [Bug 1655708] Re: target/ppc/int_helper.c:2806: strange expression ?

[Jose R. Ziviani]

** Changed in: qemu
     Assignee: (unassigned) => Jose R. Ziviani (jrziviani)

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1655708

Title:
  target/ppc/int_helper.c:2806: strange expression ?

Status in QEMU:
  New

Bug description:
  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;

  Which I read as

         zone_digit = (i * 2) ? (b->u8[BCD_DIG_BYTE(i * 2)] >> 4) :
  zone_lead;

  so I think the compiler warning is for the i * 2 lhs of the ?.

  I am not sure what to suggest as a bugfix.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1655708/+subscriptions

[Qemu-devel] [Bug 1655708] Re: target/ppc/int_helper.c:2806: strange expression ?

[dcb]

> so it is just as easy to write 'i ? ...' instead of the weirder
> '(i * 2) ? ...'.

I suspect it is just possible that the i * 2 expression is a typo
for something else, perhaps i & 2 or i << 2 or i >> 2 or something else.

I don't know the code so I am unable to offer better guidance.

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1655708

Title:
  target/ppc/int_helper.c:2806: strange expression ?

Status in QEMU:
  New

Bug description:
  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;

  Which I read as

         zone_digit = (i * 2) ? (b->u8[BCD_DIG_BYTE(i * 2)] >> 4) :
  zone_lead;

  so I think the compiler warning is for the i * 2 lhs of the ?.

  I am not sure what to suggest as a bugfix.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1655708/+subscriptions

[Qemu-devel] [Bug 1655708] Re: target/ppc/int_helper.c:2806: strange expression ?

[Thomas Huth]

Patch has been posted to the mailing list:
https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg02008.html

** Changed in: qemu
       Status: New => In Progress

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1655708

Title:
  target/ppc/int_helper.c:2806: strange expression ?

Status in QEMU:
  In Progress

Bug description:
  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;

  Which I read as

         zone_digit = (i * 2) ? (b->u8[BCD_DIG_BYTE(i * 2)] >> 4) :
  zone_lead;

  so I think the compiler warning is for the i * 2 lhs of the ?.

  I am not sure what to suggest as a bugfix.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1655708/+subscriptions

[Qemu-devel] [Bug 1655708] Re: target/ppc/int_helper.c:2806: strange expression ?

[Thomas Huth]

Fix had been committed here:
http://git.qemu.org/?p=qemu.git;a=commitdiff;h=365206aeb3d0bb72043d

** Changed in: qemu
       Status: In Progress => Fix Released

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1655708

Title:
  target/ppc/int_helper.c:2806: strange expression ?

Status in QEMU:
  Fix Released

Bug description:
  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;

  Which I read as

         zone_digit = (i * 2) ? (b->u8[BCD_DIG_BYTE(i * 2)] >> 4) :
  zone_lead;

  so I think the compiler warning is for the i * 2 lhs of the ?.

  I am not sure what to suggest as a bugfix.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1655708/+subscriptions