On 12/22/2016 08:30 AM, <email address hidden> wrote: > From: Corey Minyard <email address hidden> > > 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 <email 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 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
On 12/22/2016 08:30 AM, <email address hidden> wrote: /bugs.launchpad .net/bugs/ 1651167 isa_ipmi_ bt.c | 18 +++++++++--------- isa_ipmi_ bt.c b/hw/ipmi/ isa_ipmi_ bt.c isa_ipmi_ bt.c isa_ipmi_ bt.c GET_CLR_ WR(d) (((d) >> IPMI_BT_CLR_WR_BIT) & 0x1) SET_CLR_ WR(d, v) (d) = (((d) & ~IPMI_BT_ CLR_WR_ MASK) | \
> From: Corey Minyard <email address hidden>
>
> Macro parameters should almost always have () around them when used.
> llvm reported an error on this.
>
> Reported in https:/
>
> Signed-off-by: Corey Minyard <email address hidden>
> ---
> hw/ipmi/
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/hw/ipmi/
> index f036617..8a97314 100644
> --- a/hw/ipmi/
> +++ b/hw/ipmi/
> @@ -40,37 +40,37 @@
> #define IPMI_BT_CLR_WR_MASK (1 << IPMI_BT_CLR_WR_BIT)
> #define IPMI_BT_
> #define IPMI_BT_
Still under-parenthes ized, if the result of IPMI_BT_ SET_CLR_ WR() is used
in any larger expression;
> - (((v & 1) << IPMI_BT_ CLR_WR_ BIT))) CLR_WR_ BIT)))
> + ((((v) & 1) << IPMI_BT_
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) | \ CLR_WR_ BIT)))
(((v) & 1) << IPMI_BT_
> GET_CLR_ RD(d) (((d) >> IPMI_BT_CLR_RD_BIT) & 0x1) SET_CLR_ RD(d, v) (d) = (((d) & ~IPMI_BT_ CLR_RD_ MASK) | \ CLR_RD_ BIT))) CLR_RD_ BIT)))
> #define IPMI_BT_CLR_RD_MASK (1 << IPMI_BT_CLR_RD_BIT)
> #define IPMI_BT_
> #define IPMI_BT_
> - (((v & 1) << IPMI_BT_
> + ((((v) & 1) << IPMI_BT_
and again, throughout the patch.
-- libvirt. org
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://