On 12/22/2016 08:30 AM, miny...@acm.org wrote: > From: Corey Minyard <cminy...@mvista.com> > > Macro parameters should almost always have () around them when used. > llvm reported an error on this. > > Reported in https://bugs.launchpad.net/bugs/1651167 > > Signed-off-by: Corey Minyard <cminy...@mvista.com> > --- > 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 f036617..8a97314 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) | \
Still under-parenthesized, if the result of IPMI_BT_SET_CLR_WR() is used
in any larger expression;
> - (((v & 1) << IPMI_BT_CLR_WR_BIT)))
> + ((((v) & 1) << IPMI_BT_CLR_WR_BIT)))
and at the same time, you (still) have a redundant set on the second line.
Better would be:
((d) = (((d) & ~IPMI_BD_CLR_WR_MASK) | \
(((v) & 1) << IPMI_BT_CLR_WR_BIT)))
>
> #define IPMI_BT_CLR_RD_MASK (1 << IPMI_BT_CLR_RD_BIT)
> #define IPMI_BT_GET_CLR_RD(d) (((d) >> IPMI_BT_CLR_RD_BIT) & 0x1)
> #define IPMI_BT_SET_CLR_RD(d, v) (d) = (((d) & ~IPMI_BT_CLR_RD_MASK) | \
> - (((v & 1) << IPMI_BT_CLR_RD_BIT)))
> + ((((v) & 1) << IPMI_BT_CLR_RD_BIT)))
and again, throughout the patch.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
