All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v4] ipmi: Fix macro issues
@ 2017-03-31 14:33 minyard
  2017-03-31 15:04 ` [Qemu-devel] [PATCH for-2.9? " Eric Blake
  0 siblings, 1 reply; 4+ messages in thread
From: minyard @ 2017-03-31 14:33 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-trivial, Ed Maste, Eric Blake, Corey Minyard

From: Corey Minyard <cminyard@mvista.com>

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.

The macros were doing ((v) & 1) for a binary input, but that only works
if v == 0 or if v & 1.  Changed to !!(v) so they work for all values.

Remove some unused macros.

Reported in https://bugs.launchpad.net/bugs/1651167

Signed-off-by: Corey Minyard <cminyard@mvista.com>
---

Changes since the last revision:

  * Added the !!(v) change

 hw/ipmi/isa_ipmi_bt.c | 34 ++++++++++++----------------------
 1 file changed, 12 insertions(+), 22 deletions(-)

diff --git a/hw/ipmi/isa_ipmi_bt.c b/hw/ipmi/isa_ipmi_bt.c
index 1c69cb3..13a8c09 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) << 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) << 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) << 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) << 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) << 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) << IPMI_BT_B2H_IRQ_BIT)))
 
 typedef struct IPMIBT {
     IPMIBmc *bmc;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [PATCH for-2.9? v4] ipmi: Fix macro issues
  2017-03-31 14:33 [Qemu-devel] [PATCH v4] ipmi: Fix macro issues minyard
@ 2017-03-31 15:04 ` Eric Blake
  2017-03-31 15:09   ` Corey Minyard
  2017-03-31 17:49   ` Ed Maste
  0 siblings, 2 replies; 4+ messages in thread
From: Eric Blake @ 2017-03-31 15:04 UTC (permalink / raw)
  To: minyard, qemu-devel; +Cc: qemu-trivial, Ed Maste, Corey Minyard

[-- Attachment #1: Type: text/plain, Size: 1161 bytes --]

On 03/31/2017 09:33 AM, minyard@acm.org wrote:
> From: Corey Minyard <cminyard@mvista.com>
> 
> 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.
> 
> The macros were doing ((v) & 1) for a binary input, but that only works
> if v == 0 or if v & 1.  Changed to !!(v) so they work for all values.

s/&/==/

> 
> Remove some unused macros.
> 
> Reported in https://bugs.launchpad.net/bugs/1651167

Might also be worth adding that an audit of the code finds no semantic
change, that this is just cleaning up the compiler warning.

> 
> Signed-off-by: Corey Minyard <cminyard@mvista.com>
> ---
> 
> Changes since the last revision:
> 
>   * Added the !!(v) change
> 
>  hw/ipmi/isa_ipmi_bt.c | 34 ++++++++++++----------------------
>  1 file changed, 12 insertions(+), 22 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [PATCH for-2.9? v4] ipmi: Fix macro issues
  2017-03-31 15:04 ` [Qemu-devel] [PATCH for-2.9? " Eric Blake
@ 2017-03-31 15:09   ` Corey Minyard
  2017-03-31 17:49   ` Ed Maste
  1 sibling, 0 replies; 4+ messages in thread
From: Corey Minyard @ 2017-03-31 15:09 UTC (permalink / raw)
  To: Eric Blake, minyard, qemu-devel; +Cc: qemu-trivial, Ed Maste

On 03/31/2017 10:04 AM, Eric Blake wrote:
> On 03/31/2017 09:33 AM, minyard@acm.org wrote:
>> From: Corey Minyard <cminyard@mvista.com>
>>
>> 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.
>>
>> The macros were doing ((v) & 1) for a binary input, but that only works
>> if v == 0 or if v & 1.  Changed to !!(v) so they work for all values.
> s/&/==/
>

I originally wrote v == 1, but I realized that it worked with anything 
where the
bottom bit of v was set.  Thus, v & 1.

>> Remove some unused macros.
>>
>> Reported in https://bugs.launchpad.net/bugs/1651167
> Might also be worth adding that an audit of the code finds no semantic
> change, that this is just cleaning up the compiler warning.

Will do.

Thanks,

-corey
>
>> Signed-off-by: Corey Minyard <cminyard@mvista.com>
>> ---
>>
>> Changes since the last revision:
>>
>>    * Added the !!(v) change
>>
>>   hw/ipmi/isa_ipmi_bt.c | 34 ++++++++++++----------------------
>>   1 file changed, 12 insertions(+), 22 deletions(-)
> Reviewed-by: Eric Blake <eblake@redhat.com>
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [PATCH for-2.9? v4] ipmi: Fix macro issues
  2017-03-31 15:04 ` [Qemu-devel] [PATCH for-2.9? " Eric Blake
  2017-03-31 15:09   ` Corey Minyard
@ 2017-03-31 17:49   ` Ed Maste
  1 sibling, 0 replies; 4+ messages in thread
From: Ed Maste @ 2017-03-31 17:49 UTC (permalink / raw)
  To: Eric Blake; +Cc: Corey Minyard, qemu-devel, qemu-trivial, Corey Minyard

On 31 March 2017 at 11:04, Eric Blake <eblake@redhat.com> wrote:
>
> Might also be worth adding that an audit of the code finds no semantic
> change, that this is just cleaning up the compiler warning.

We should include core detail of your detailed analysis in such a
statement I think, because I it's more than just cleaning up a
warning. For me at least the warning pointed out that the expression
is not parsed would be expected (if this were a function and not a
macro), and it's just because of constraints on the inputs that
there's no functional change.

>> Signed-off-by: Corey Minyard <cminyard@mvista.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Ed Maste <emaste@freebsd.org>

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2017-03-31 17:49 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-31 14:33 [Qemu-devel] [PATCH v4] ipmi: Fix macro issues minyard
2017-03-31 15:04 ` [Qemu-devel] [PATCH for-2.9? " Eric Blake
2017-03-31 15:09   ` Corey Minyard
2017-03-31 17:49   ` Ed Maste

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.