diff options
Diffstat (limited to 'results/classifier/118/graphic/1651167')
| -rw-r--r-- | results/classifier/118/graphic/1651167 | 580 |
1 files changed, 580 insertions, 0 deletions
diff --git a/results/classifier/118/graphic/1651167 b/results/classifier/118/graphic/1651167 new file mode 100644 index 000000000..13bbd0bcd --- /dev/null +++ b/results/classifier/118/graphic/1651167 @@ -0,0 +1,580 @@ +graphic: 0.961 +architecture: 0.950 +permissions: 0.939 +device: 0.930 +register: 0.924 +debug: 0.915 +performance: 0.913 +assembly: 0.900 +arm: 0.899 +user-level: 0.894 +socket: 0.890 +virtual: 0.884 +vnc: 0.866 +TCG: 0.851 +PID: 0.846 +kernel: 0.845 +risc-v: 0.843 +VMM: 0.841 +files: 0.829 +hypervisor: 0.823 +KVM: 0.810 +network: 0.805 +semantic: 0.799 +mistranslation: 0.777 +boot: 0.773 +peripherals: 0.740 +ppc: 0.716 +x86: 0.598 +i386: 0.294 + +hw/ipmi/isa_ipmi_bt.c:283: suspect use of macro ? + +I just had a go at compiling qemu trunk with +llvm trunk. It said: + +hw/ipmi/isa_ipmi_bt.c:283:31: warning: logical not is only applied to the left hand side of this bitwise operator [-Wlogical-not-parentheses] + +Source code is + + IPMI_BT_SET_HBUSY(ib->control_reg, + !IPMI_BT_GET_HBUSY(ib->control_reg)); + +That use of ! causes trouble. The SET and GET +macros are defined as: + +#define IPMI_BT_GET_HBUSY(d) (((d) >> IPMI_BT_HBUSY_BIT) & 0x1) +#define IPMI_BT_SET_HBUSY(d, v) (d) = (((d) & ~IPMI_BT_HBUSY_MASK) | \ + (((v & 1) << IPMI_BT_HBUSY_BIT))) + +I can make the compiler shut up by adding extra () in the last +use of v in the SET macro, like this: + +#define IPMI_BT_SET_HBUSY(d, v) (d) = (((d) & ~IPMI_BT_HBUSY_MASK) | \ + ((((v) & 1) << IPMI_BT_HBUSY_BIT))) + +I think this is standard good practice when using macro parameters anyway. + +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) | \ +- (((v & 1) << IPMI_BT_CLR_WR_BIT))) ++ ((((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))) + + #define IPMI_BT_H2B_ATN_MASK (1 << IPMI_BT_H2B_ATN_BIT) + #define IPMI_BT_GET_H2B_ATN(d) (((d) >> IPMI_BT_H2B_ATN_BIT) & 0x1) + #define IPMI_BT_SET_H2B_ATN(d, v) (d) = (((d) & ~IPMI_BT_H2B_ATN_MASK) | \ +- (((v & 1) << IPMI_BT_H2B_ATN_BIT))) ++ ((((v) & 1) << IPMI_BT_H2B_ATN_BIT))) + + #define IPMI_BT_B2H_ATN_MASK (1 << IPMI_BT_B2H_ATN_BIT) + #define IPMI_BT_GET_B2H_ATN(d) (((d) >> IPMI_BT_B2H_ATN_BIT) & 0x1) + #define IPMI_BT_SET_B2H_ATN(d, v) (d) = (((d) & ~IPMI_BT_B2H_ATN_MASK) | \ +- (((v & 1) << IPMI_BT_B2H_ATN_BIT))) ++ ((((v) & 1) << IPMI_BT_B2H_ATN_BIT))) + + #define IPMI_BT_SMS_ATN_MASK (1 << IPMI_BT_SMS_ATN_BIT) + #define IPMI_BT_GET_SMS_ATN(d) (((d) >> IPMI_BT_SMS_ATN_BIT) & 0x1) + #define IPMI_BT_SET_SMS_ATN(d, v) (d) = (((d) & ~IPMI_BT_SMS_ATN_MASK) | \ +- (((v & 1) << IPMI_BT_SMS_ATN_BIT))) ++ ((((v) & 1) << IPMI_BT_SMS_ATN_BIT))) + + #define IPMI_BT_HBUSY_MASK (1 << IPMI_BT_HBUSY_BIT) + #define IPMI_BT_GET_HBUSY(d) (((d) >> IPMI_BT_HBUSY_BIT) & 0x1) + #define IPMI_BT_SET_HBUSY(d, v) (d) = (((d) & ~IPMI_BT_HBUSY_MASK) | \ +- (((v & 1) << IPMI_BT_HBUSY_BIT))) ++ ((((v) & 1) << IPMI_BT_HBUSY_BIT))) + + #define IPMI_BT_BBUSY_MASK (1 << IPMI_BT_BBUSY_BIT) + #define IPMI_BT_GET_BBUSY(d) (((d) >> IPMI_BT_BBUSY_BIT) & 0x1) + #define IPMI_BT_SET_BBUSY(d, v) (d) = (((d) & ~IPMI_BT_BBUSY_MASK) | \ +- (((v & 1) << IPMI_BT_BBUSY_BIT))) ++ ((((v) & 1) << IPMI_BT_BBUSY_BIT))) + + + /* Mask register */ +@@ -80,12 +80,12 @@ + #define IPMI_BT_B2H_IRQ_EN_MASK (1 << IPMI_BT_B2H_IRQ_EN_BIT) + #define IPMI_BT_GET_B2H_IRQ_EN(d) (((d) >> IPMI_BT_B2H_IRQ_EN_BIT) & 0x1) + #define IPMI_BT_SET_B2H_IRQ_EN(d, v) (d) = (((d) & ~IPMI_BT_B2H_IRQ_EN_MASK) | \ +- (((v & 1) << IPMI_BT_B2H_IRQ_EN_BIT))) ++ ((((v) & 1) << IPMI_BT_B2H_IRQ_EN_BIT))) + + #define IPMI_BT_B2H_IRQ_MASK (1 << IPMI_BT_B2H_IRQ_BIT) + #define IPMI_BT_GET_B2H_IRQ(d) (((d) >> IPMI_BT_B2H_IRQ_BIT) & 0x1) + #define IPMI_BT_SET_B2H_IRQ(d, v) (d) = (((d) & ~IPMI_BT_B2H_IRQ_MASK) | \ +- (((v & 1) << IPMI_BT_B2H_IRQ_BIT))) ++ ((((v) & 1) << IPMI_BT_B2H_IRQ_BIT))) + + typedef struct IPMIBT { + IPMIBmc *bmc; +-- +2.7.4 + + + +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 09:01 AM, Eric Blake wrote: +> 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; + +I wasn't thinking about this being used in a larger expression, but it +should be protected, +I suppose. I'll re-submit with that fixed and the extra () removed. + +Thanks, + +-corey + +>> - (((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. +> +> + + + +From: Corey Minyard <email address hidden> + +Macro parameters should almost always have () around them when used. +llvm reported an error on this. + +Remove redundant parenthesis and put parenthesis around the entire +macros with assignments in case they are used in an expression. + +Remove some unused macros. + +Reported in https://bugs.launchpad.net/bugs/1651167 + +Signed-off-by: Corey Minyard <email address hidden> +--- + hw/ipmi/isa_ipmi_bt.c | 34 ++++++++++++---------------------- + 1 file changed, 12 insertions(+), 22 deletions(-) + +Changes in v2: + * Put parenthesis around macros that had assignment in them. + * Removed some redundant parenthesis. + * Removed some macros that were not used. + +diff --git a/hw/ipmi/isa_ipmi_bt.c b/hw/ipmi/isa_ipmi_bt.c +index f036617..68bf5cd 100644 +--- a/hw/ipmi/isa_ipmi_bt.c ++++ b/hw/ipmi/isa_ipmi_bt.c +@@ -37,40 +37,30 @@ + #define IPMI_BT_HBUSY_BIT 6 + #define IPMI_BT_BBUSY_BIT 7 + +-#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))) + +-#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))) + +-#define IPMI_BT_H2B_ATN_MASK (1 << IPMI_BT_H2B_ATN_BIT) + #define IPMI_BT_GET_H2B_ATN(d) (((d) >> IPMI_BT_H2B_ATN_BIT) & 0x1) +-#define IPMI_BT_SET_H2B_ATN(d, v) (d) = (((d) & ~IPMI_BT_H2B_ATN_MASK) | \ +- (((v & 1) << IPMI_BT_H2B_ATN_BIT))) + + #define IPMI_BT_B2H_ATN_MASK (1 << IPMI_BT_B2H_ATN_BIT) + #define IPMI_BT_GET_B2H_ATN(d) (((d) >> IPMI_BT_B2H_ATN_BIT) & 0x1) +-#define IPMI_BT_SET_B2H_ATN(d, v) (d) = (((d) & ~IPMI_BT_B2H_ATN_MASK) | \ +- (((v & 1) << IPMI_BT_B2H_ATN_BIT))) ++#define IPMI_BT_SET_B2H_ATN(d, v) ((d) = (((d) & ~IPMI_BT_B2H_ATN_MASK) | \ ++ (((v) & 1) << IPMI_BT_B2H_ATN_BIT))) + + #define IPMI_BT_SMS_ATN_MASK (1 << IPMI_BT_SMS_ATN_BIT) + #define IPMI_BT_GET_SMS_ATN(d) (((d) >> IPMI_BT_SMS_ATN_BIT) & 0x1) +-#define IPMI_BT_SET_SMS_ATN(d, v) (d) = (((d) & ~IPMI_BT_SMS_ATN_MASK) | \ +- (((v & 1) << IPMI_BT_SMS_ATN_BIT))) ++#define IPMI_BT_SET_SMS_ATN(d, v) ((d) = (((d) & ~IPMI_BT_SMS_ATN_MASK) | \ ++ (((v) & 1) << IPMI_BT_SMS_ATN_BIT))) + + #define IPMI_BT_HBUSY_MASK (1 << IPMI_BT_HBUSY_BIT) + #define IPMI_BT_GET_HBUSY(d) (((d) >> IPMI_BT_HBUSY_BIT) & 0x1) +-#define IPMI_BT_SET_HBUSY(d, v) (d) = (((d) & ~IPMI_BT_HBUSY_MASK) | \ +- (((v & 1) << IPMI_BT_HBUSY_BIT))) ++#define IPMI_BT_SET_HBUSY(d, v) ((d) = (((d) & ~IPMI_BT_HBUSY_MASK) | \ ++ (((v) & 1) << IPMI_BT_HBUSY_BIT))) + + #define IPMI_BT_BBUSY_MASK (1 << IPMI_BT_BBUSY_BIT) +-#define IPMI_BT_GET_BBUSY(d) (((d) >> IPMI_BT_BBUSY_BIT) & 0x1) +-#define IPMI_BT_SET_BBUSY(d, v) (d) = (((d) & ~IPMI_BT_BBUSY_MASK) | \ +- (((v & 1) << IPMI_BT_BBUSY_BIT))) ++#define IPMI_BT_SET_BBUSY(d, v) ((d) = (((d) & ~IPMI_BT_BBUSY_MASK) | \ ++ (((v) & 1) << IPMI_BT_BBUSY_BIT))) + + + /* Mask register */ +@@ -79,13 +69,13 @@ + + #define IPMI_BT_B2H_IRQ_EN_MASK (1 << IPMI_BT_B2H_IRQ_EN_BIT) + #define IPMI_BT_GET_B2H_IRQ_EN(d) (((d) >> IPMI_BT_B2H_IRQ_EN_BIT) & 0x1) +-#define IPMI_BT_SET_B2H_IRQ_EN(d, v) (d) = (((d) & ~IPMI_BT_B2H_IRQ_EN_MASK) | \ +- (((v & 1) << IPMI_BT_B2H_IRQ_EN_BIT))) ++#define IPMI_BT_SET_B2H_IRQ_EN(d, v) ((d) = (((d) & ~IPMI_BT_B2H_IRQ_EN_MASK) |\ ++ (((v) & 1) << IPMI_BT_B2H_IRQ_EN_BIT))) + + #define IPMI_BT_B2H_IRQ_MASK (1 << IPMI_BT_B2H_IRQ_BIT) + #define IPMI_BT_GET_B2H_IRQ(d) (((d) >> IPMI_BT_B2H_IRQ_BIT) & 0x1) +-#define IPMI_BT_SET_B2H_IRQ(d, v) (d) = (((d) & ~IPMI_BT_B2H_IRQ_MASK) | \ +- (((v & 1) << IPMI_BT_B2H_IRQ_BIT))) ++#define IPMI_BT_SET_B2H_IRQ(d, v) ((d) = (((d) & ~IPMI_BT_B2H_IRQ_MASK) | \ ++ (((v) & 1) << IPMI_BT_B2H_IRQ_BIT))) + + typedef struct IPMIBT { + IPMIBmc *bmc; +-- +2.7.4 + + + +On 12/22/2016 01:18 PM, <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. +> +> Remove redundant parenthesis and put parenthesis around the entire +> macros with assignments in case they are used in an expression. +> +> Remove some unused macros. +> +> Reported in https://bugs.launchpad.net/bugs/1651167 +> +> Signed-off-by: Corey Minyard <email address hidden> +> --- +> hw/ipmi/isa_ipmi_bt.c | 34 ++++++++++++---------------------- +> 1 file changed, 12 insertions(+), 22 deletions(-) + +Reviewed-by: Eric Blake <email address hidden> + +-- +Eric Blake eblake redhat com +1-919-301-3266 +Libvirt virtualization library http://libvirt.org + + + +From: Corey Minyard <email address hidden> + +Macro parameters should almost always have () around them when used. +llvm reported an error on this. + +Remove redundant parenthesis and put parenthesis around the entire +macros with assignments in case they are used in an expression. + +Remove some unused macros. + +Reported in https://bugs.launchpad.net/bugs/1651167 + +Signed-off-by: Corey Minyard <email address hidden> +Reviewed-by: Eric Blake <email address hidden> +--- + hw/ipmi/isa_ipmi_bt.c | 34 ++++++++++++---------------------- + 1 file changed, 12 insertions(+), 22 deletions(-) + +Changed in v3: + * Add Eric's reviewed-by. Thanks! + +Changes in v2: + * Put parenthesis around macros that had assignment in them. + * Removed some redundant parenthesis. + * Removed some macros that were not used. + +diff --git a/hw/ipmi/isa_ipmi_bt.c b/hw/ipmi/isa_ipmi_bt.c +index f036617..68bf5cd 100644 +--- a/hw/ipmi/isa_ipmi_bt.c ++++ b/hw/ipmi/isa_ipmi_bt.c +@@ -37,40 +37,30 @@ + #define IPMI_BT_HBUSY_BIT 6 + #define IPMI_BT_BBUSY_BIT 7 + +-#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))) + +-#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))) + +-#define IPMI_BT_H2B_ATN_MASK (1 << IPMI_BT_H2B_ATN_BIT) + #define IPMI_BT_GET_H2B_ATN(d) (((d) >> IPMI_BT_H2B_ATN_BIT) & 0x1) +-#define IPMI_BT_SET_H2B_ATN(d, v) (d) = (((d) & ~IPMI_BT_H2B_ATN_MASK) | \ +- (((v & 1) << IPMI_BT_H2B_ATN_BIT))) + + #define IPMI_BT_B2H_ATN_MASK (1 << IPMI_BT_B2H_ATN_BIT) + #define IPMI_BT_GET_B2H_ATN(d) (((d) >> IPMI_BT_B2H_ATN_BIT) & 0x1) +-#define IPMI_BT_SET_B2H_ATN(d, v) (d) = (((d) & ~IPMI_BT_B2H_ATN_MASK) | \ +- (((v & 1) << IPMI_BT_B2H_ATN_BIT))) ++#define IPMI_BT_SET_B2H_ATN(d, v) ((d) = (((d) & ~IPMI_BT_B2H_ATN_MASK) | \ ++ (((v) & 1) << IPMI_BT_B2H_ATN_BIT))) + + #define IPMI_BT_SMS_ATN_MASK (1 << IPMI_BT_SMS_ATN_BIT) + #define IPMI_BT_GET_SMS_ATN(d) (((d) >> IPMI_BT_SMS_ATN_BIT) & 0x1) +-#define IPMI_BT_SET_SMS_ATN(d, v) (d) = (((d) & ~IPMI_BT_SMS_ATN_MASK) | \ +- (((v & 1) << IPMI_BT_SMS_ATN_BIT))) ++#define IPMI_BT_SET_SMS_ATN(d, v) ((d) = (((d) & ~IPMI_BT_SMS_ATN_MASK) | \ ++ (((v) & 1) << IPMI_BT_SMS_ATN_BIT))) + + #define IPMI_BT_HBUSY_MASK (1 << IPMI_BT_HBUSY_BIT) + #define IPMI_BT_GET_HBUSY(d) (((d) >> IPMI_BT_HBUSY_BIT) & 0x1) +-#define IPMI_BT_SET_HBUSY(d, v) (d) = (((d) & ~IPMI_BT_HBUSY_MASK) | \ +- (((v & 1) << IPMI_BT_HBUSY_BIT))) ++#define IPMI_BT_SET_HBUSY(d, v) ((d) = (((d) & ~IPMI_BT_HBUSY_MASK) | \ ++ (((v) & 1) << IPMI_BT_HBUSY_BIT))) + + #define IPMI_BT_BBUSY_MASK (1 << IPMI_BT_BBUSY_BIT) +-#define IPMI_BT_GET_BBUSY(d) (((d) >> IPMI_BT_BBUSY_BIT) & 0x1) +-#define IPMI_BT_SET_BBUSY(d, v) (d) = (((d) & ~IPMI_BT_BBUSY_MASK) | \ +- (((v & 1) << IPMI_BT_BBUSY_BIT))) ++#define IPMI_BT_SET_BBUSY(d, v) ((d) = (((d) & ~IPMI_BT_BBUSY_MASK) | \ ++ (((v) & 1) << IPMI_BT_BBUSY_BIT))) + + + /* Mask register */ +@@ -79,13 +69,13 @@ + + #define IPMI_BT_B2H_IRQ_EN_MASK (1 << IPMI_BT_B2H_IRQ_EN_BIT) + #define IPMI_BT_GET_B2H_IRQ_EN(d) (((d) >> IPMI_BT_B2H_IRQ_EN_BIT) & 0x1) +-#define IPMI_BT_SET_B2H_IRQ_EN(d, v) (d) = (((d) & ~IPMI_BT_B2H_IRQ_EN_MASK) | \ +- (((v & 1) << IPMI_BT_B2H_IRQ_EN_BIT))) ++#define IPMI_BT_SET_B2H_IRQ_EN(d, v) ((d) = (((d) & ~IPMI_BT_B2H_IRQ_EN_MASK) |\ ++ (((v) & 1) << IPMI_BT_B2H_IRQ_EN_BIT))) + + #define IPMI_BT_B2H_IRQ_MASK (1 << IPMI_BT_B2H_IRQ_BIT) + #define IPMI_BT_GET_B2H_IRQ(d) (((d) >> IPMI_BT_B2H_IRQ_BIT) & 0x1) +-#define IPMI_BT_SET_B2H_IRQ(d, v) (d) = (((d) & ~IPMI_BT_B2H_IRQ_MASK) | \ +- (((v & 1) << IPMI_BT_B2H_IRQ_BIT))) ++#define IPMI_BT_SET_B2H_IRQ(d, v) ((d) = (((d) & ~IPMI_BT_B2H_IRQ_MASK) | \ ++ (((v) & 1) << IPMI_BT_B2H_IRQ_BIT))) + + typedef struct IPMIBT { + IPMIBmc *bmc; +-- +2.7.4 + + + +Ping - did this ever get applied? + +On 12/23/2016 08:07 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. +> +> Remove redundant parenthesis and put parenthesis around the entire +> macros with assignments in case they are used in an expression. +> +> Remove some unused macros. +> +> Reported in https://bugs.launchpad.net/bugs/1651167 +> +> Signed-off-by: Corey Minyard <email address hidden> +> Reviewed-by: Eric Blake <email address hidden> +> --- +> hw/ipmi/isa_ipmi_bt.c | 34 ++++++++++++---------------------- +> 1 file changed, 12 insertions(+), 22 deletions(-) +> +> Changed in v3: +> * Add Eric's reviewed-by. Thanks! +> +> Changes in v2: +> * Put parenthesis around macros that had assignment in them. +> * Removed some redundant parenthesis. +> * Removed some macros that were not used. +> +> diff --git a/hw/ipmi/isa_ipmi_bt.c b/hw/ipmi/isa_ipmi_bt.c +> index f036617..68bf5cd 100644 +> --- a/hw/ipmi/isa_ipmi_bt.c +> +++ b/hw/ipmi/isa_ipmi_bt.c +> @@ -37,40 +37,30 @@ +> #define IPMI_BT_HBUSY_BIT 6 +> #define IPMI_BT_BBUSY_BIT 7 +> +> -#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))) +> +> -#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))) +> +> -#define IPMI_BT_H2B_ATN_MASK (1 << IPMI_BT_H2B_ATN_BIT) +> #define IPMI_BT_GET_H2B_ATN(d) (((d) >> IPMI_BT_H2B_ATN_BIT) & 0x1) +> -#define IPMI_BT_SET_H2B_ATN(d, v) (d) = (((d) & ~IPMI_BT_H2B_ATN_MASK) | \ +> - (((v & 1) << IPMI_BT_H2B_ATN_BIT))) +> +> #define IPMI_BT_B2H_ATN_MASK (1 << IPMI_BT_B2H_ATN_BIT) +> #define IPMI_BT_GET_B2H_ATN(d) (((d) >> IPMI_BT_B2H_ATN_BIT) & 0x1) +> -#define IPMI_BT_SET_B2H_ATN(d, v) (d) = (((d) & ~IPMI_BT_B2H_ATN_MASK) | \ +> - (((v & 1) << IPMI_BT_B2H_ATN_BIT))) +> +#define IPMI_BT_SET_B2H_ATN(d, v) ((d) = (((d) & ~IPMI_BT_B2H_ATN_MASK) | \ +> + (((v) & 1) << IPMI_BT_B2H_ATN_BIT))) +> +> #define IPMI_BT_SMS_ATN_MASK (1 << IPMI_BT_SMS_ATN_BIT) +> #define IPMI_BT_GET_SMS_ATN(d) (((d) >> IPMI_BT_SMS_ATN_BIT) & 0x1) +> -#define IPMI_BT_SET_SMS_ATN(d, v) (d) = (((d) & ~IPMI_BT_SMS_ATN_MASK) | \ +> - (((v & 1) << IPMI_BT_SMS_ATN_BIT))) +> +#define IPMI_BT_SET_SMS_ATN(d, v) ((d) = (((d) & ~IPMI_BT_SMS_ATN_MASK) | \ +> + (((v) & 1) << IPMI_BT_SMS_ATN_BIT))) +> +> #define IPMI_BT_HBUSY_MASK (1 << IPMI_BT_HBUSY_BIT) +> #define IPMI_BT_GET_HBUSY(d) (((d) >> IPMI_BT_HBUSY_BIT) & 0x1) +> -#define IPMI_BT_SET_HBUSY(d, v) (d) = (((d) & ~IPMI_BT_HBUSY_MASK) | \ +> - (((v & 1) << IPMI_BT_HBUSY_BIT))) +> +#define IPMI_BT_SET_HBUSY(d, v) ((d) = (((d) & ~IPMI_BT_HBUSY_MASK) | \ +> + (((v) & 1) << IPMI_BT_HBUSY_BIT))) +> +> #define IPMI_BT_BBUSY_MASK (1 << IPMI_BT_BBUSY_BIT) +> -#define IPMI_BT_GET_BBUSY(d) (((d) >> IPMI_BT_BBUSY_BIT) & 0x1) +> -#define IPMI_BT_SET_BBUSY(d, v) (d) = (((d) & ~IPMI_BT_BBUSY_MASK) | \ +> - (((v & 1) << IPMI_BT_BBUSY_BIT))) +> +#define IPMI_BT_SET_BBUSY(d, v) ((d) = (((d) & ~IPMI_BT_BBUSY_MASK) | \ +> + (((v) & 1) << IPMI_BT_BBUSY_BIT))) +> +> +> /* Mask register */ +> @@ -79,13 +69,13 @@ +> +> #define IPMI_BT_B2H_IRQ_EN_MASK (1 << IPMI_BT_B2H_IRQ_EN_BIT) +> #define IPMI_BT_GET_B2H_IRQ_EN(d) (((d) >> IPMI_BT_B2H_IRQ_EN_BIT) & 0x1) +> -#define IPMI_BT_SET_B2H_IRQ_EN(d, v) (d) = (((d) & ~IPMI_BT_B2H_IRQ_EN_MASK) | \ +> - (((v & 1) << IPMI_BT_B2H_IRQ_EN_BIT))) +> +#define IPMI_BT_SET_B2H_IRQ_EN(d, v) ((d) = (((d) & ~IPMI_BT_B2H_IRQ_EN_MASK) |\ +> + (((v) & 1) << IPMI_BT_B2H_IRQ_EN_BIT))) +> +> #define IPMI_BT_B2H_IRQ_MASK (1 << IPMI_BT_B2H_IRQ_BIT) +> #define IPMI_BT_GET_B2H_IRQ(d) (((d) >> IPMI_BT_B2H_IRQ_BIT) & 0x1) +> -#define IPMI_BT_SET_B2H_IRQ(d, v) (d) = (((d) & ~IPMI_BT_B2H_IRQ_MASK) | \ +> - (((v & 1) << IPMI_BT_B2H_IRQ_BIT))) +> +#define IPMI_BT_SET_B2H_IRQ(d, v) ((d) = (((d) & ~IPMI_BT_B2H_IRQ_MASK) | \ +> + (((v) & 1) << IPMI_BT_B2H_IRQ_BIT))) +> +> typedef struct IPMIBT { +> IPMIBmc *bmc; +> + +-- +Eric Blake eblake redhat com +1-919-301-3266 +Libvirt virtualization library http://libvirt.org + + + +Fix has been included here: +http://git.qemu.org/?p=qemu.git;a=commitdiff;h=cb9a05a4f169347f85 + |