[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-trivial] [Qemu-devel] [PATCH] ipmi: use parens to avoid unexpe
From: |
Eric Blake |
Subject: |
Re: [Qemu-trivial] [Qemu-devel] [PATCH] ipmi: use parens to avoid unexpected macro var evaluation |
Date: |
Thu, 30 Mar 2017 11:09:37 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 |
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