Re: [Qemu-trivial] [Qemu-devel] [PATCH] ipmi: use parens to avoid unexpected macro var evaluation

[Eric Blake]

On 03/30/2017 06:22 PM, Ed Maste wrote:
> Found via Clang warning: logical not is only applied to the left hand
> side of this bitwise operator [-Wlogical-not-parentheses]
> 
>                               !IPMI_BT_GET_HBUSY(ib->control_reg));
>                               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

You just re-found bug 1651167 reported in December:
https://lists.gnu.org/archive/html/qemu-devel/2016-12/msg02704.html

> 
> expanded from macro 'IPMI_BT_SET_HBUSY'
>                                        (((v & 1) << IPMI_BT_HBUSY_BIT)))
>                                           ^ ~
> 
> Signed-off-by: Ed Maste <address@hidden>
> ---
>  hw/ipmi/isa_ipmi_bt.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/ipmi/isa_ipmi_bt.c b/hw/ipmi/isa_ipmi_bt.c
> index 1c69cb33f8..c1dea503a2 100644
> --- a/hw/ipmi/isa_ipmi_bt.c
> +++ b/hw/ipmi/isa_ipmi_bt.c
> @@ -40,37 +40,37 @@
>  #define IPMI_BT_CLR_WR_MASK        (1 << IPMI_BT_CLR_WR_BIT)
>  #define IPMI_BT_GET_CLR_WR(d)      (((d) >> IPMI_BT_CLR_WR_BIT) & 0x1)
>  #define IPMI_BT_SET_CLR_WR(d, v)   (d) = (((d) & ~IPMI_BT_CLR_WR_MASK) | \
> -                                       (((v & 1) << IPMI_BT_CLR_WR_BIT)))
> +                                       ((((v) & 1) << IPMI_BT_CLR_WR_BIT)))

and still overparenthesized.  My proposal back then was:
https://lists.gnu.org/archive/html/qemu-devel/2016-12/msg03095.html

> Better would be:
> 
> ((d) = (((d) & ~IPMI_BD_CLR_WR_MASK) | \
>         (((v) & 1) << IPMI_BT_CLR_WR_BIT)))

So why didn't we ever take v3 of the patch back then?
https://lists.gnu.org/archive/html/qemu-devel/2016-12/msg03196.html

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

signature.asc
Description: OpenPGP digital signature