Skip to contentSkip to conversation

nbd-client: Use correct macro parenthesization

PATCH Applied Eric Blake 4 participants 7 messages Sep 18, 2017 – Sep 24, 2017 Block layer

lore
Share & tools
lore

Eric Blake

author
#0 lore raw

PATCH 1/1 nbd-client: Use correct macro parenthesization

If 'bs' is a complex expression, we were only casting the front half rather than the full expression. Luckily, none of the callers were passing bad arguments, but it's better to be robust up front.

Signed-off-by Eric Blake <[email protected]>

---

I plan to take this through my NBD queue, unless it is picked up by qemu-trivial first...

 block/nbd-client.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
block/nbd-client.c +2 -2
index 5f241ecc22..2a528dd217 100644
@@ -31,8 +31,8 @@
 #include "qapi/error.h"
 #include "nbd-client.h"
 
-#define HANDLE_TO_INDEX(bs, handle) ((handle) ^ ((uint64_t)(intptr_t)bs))
-#define INDEX_TO_HANDLE(bs, index)  ((index)  ^ ((uint64_t)(intptr_t)bs))
+#define HANDLE_TO_INDEX(bs, handle) ((handle) ^ (uint64_t)(intptr_t)(bs))
+#define INDEX_TO_HANDLE(bs, index)  ((index)  ^ (uint64_t)(intptr_t)(bs))
 
 static void nbd_recv_coroutines_wake_all(NBDClientSession *s)
 {
Signature · 1 line
2.13.5
Reviewed-by on v1 1/1
#1 lore raw

Hi Eric,

Quoted 5 lines · Eric Blake

If 'bs' is a complex expression, we were only casting the front half rather than the full expression. Luckily, none of the callers were passing bad arguments, but it's better to be robust up front.

Signed-off-by Eric Blake <[email protected]>
Reviewed-by Philippe Mathieu-Daudé <[email protected]>

Any magic cocci script to verify there aren't no more?

Quoted 24 lines

---

I plan to take this through my NBD queue, unless it is picked up by qemu-trivial first...

  block/nbd-client.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)
block/nbd-client.c +2 -2
index 5f241ecc22..2a528dd217 100644
@@ -31,8 +31,8 @@
  #include "qapi/error.h"
  #include "nbd-client.h"
 
-#define HANDLE_TO_INDEX(bs, handle) ((handle) ^ ((uint64_t)(intptr_t)bs))
-#define INDEX_TO_HANDLE(bs, index)  ((index)  ^ ((uint64_t)(intptr_t)bs))
+#define HANDLE_TO_INDEX(bs, handle) ((handle) ^ (uint64_t)(intptr_t)(bs))
+#define INDEX_TO_HANDLE(bs, index)  ((index)  ^ (uint64_t)(intptr_t)(bs))
 
  static void nbd_recv_coroutines_wake_all(NBDClientSession *s)
  {
author
#2 lore raw
Quoted 7 lines · Philippe Mathieu-Daudé

Hi Eric,

Quoted 4 lines · Eric Blake

If 'bs' is a complex expression, we were only casting the front half rather than the full expression.  Luckily, none of the callers were passing bad arguments, but it's better to be robust up front.

Quoted 5 lines
Quoted 1 line
Signed-off-by Eric Blake <[email protected]>
Reviewed-by Philippe Mathieu-Daudé <[email protected]>

Any magic cocci script to verify there aren't no more?

I don't know if cocci can do it; checkpatch tries to check whether macro arguments are parenthesized, but even that's prone to missing things. I just spotted this particular one while reviewing a related patch.

Signature · 3 lines
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org
#3 lore raw
Quoted 6 lines · Eric Blake
Quoted 1 line · Philippe Mathieu-Daudé

Any magic cocci script to verify there aren't no more?

I don't know if cocci can do it; checkpatch tries to check whether macro arguments are parenthesized, but even that's prone to missing things. I just spotted this particular one while reviewing a related patch.

couldn't figure out.

grep -E '^\s*#define\s+\w+\([^\)]+\)\s+.*\(\w+\)\w+' (no multiline) found those unharmful:

target/mips/dsp_helper.c:#define MIPSDSP_RETURN64_16(a, b, c, d) (((uint64_t)a << 48) | \
target/mips/dsp_helper.c:#define MIPSDSP_RETURN64_32(a, b) (((uint64_t)a << 32) | (uint64_t)b) include/hw/ppc/pnv_xscom.h:#define PNV_XSCOM_EX_CORE_BASE(base, i) (base | (((uint64_t)i) << 24))

author
#4 lore raw
Quoted 13 lines · Philippe Mathieu-Daudé
Quoted 6 lines · Eric Blake
Quoted 1 line · Philippe Mathieu-Daudé

Any magic cocci script to verify there aren't no more?

I don't know if cocci can do it; checkpatch tries to check whether macro arguments are parenthesized, but even that's prone to missing things.  I just spotted this particular one while reviewing a related patch.

couldn't figure out.

grep -E '^\s*#define\s+\w+\([^\)]+\)\s+.*\(\w+\)\w+' (no multiline) found those unharmful:

Quoted 6 lines

target/mips/dsp_helper.c:#define MIPSDSP_RETURN64_16(a, b, c, d) (((uint64_t)a << 48) | \
target/mips/dsp_helper.c:#define MIPSDSP_RETURN64_32(a, b) (((uint64_t)a << 32) | (uint64_t)b)
include/hw/ppc/pnv_xscom.h:#define PNV_XSCOM_EX_CORE_BASE(base, i) (base | (((uint64_t)i) << 24))

There's probably more, but I submitted patches for these two files as separate threads for qemu-trivial.

Signature · 3 lines
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org
Reviewed-by on v1 1/1
#5 lore raw
In reply to Eric Blake
Quoted 4 lines · Eric Blake

If 'bs' is a complex expression, we were only casting the front half rather than the full expression. Luckily, none of the callers were passing bad arguments, but it's better to be robust up front.

Quoted 8 lines
Signed-off-by Eric Blake <[email protected]>

---

I plan to take this through my NBD queue, unless it is picked up by qemu-trivial first...

 block/nbd-client.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Reviewed-by Stefan Hajnoczi <[email protected]>
Applied
#6 lore raw
In reply to Eric Blake
Quoted 3 lines · Eric Blake

If 'bs' is a complex expression, we were only casting the front half rather than the full expression. Luckily, none of the callers were passing bad arguments, but it's better to be robust up front.

Applied to -trivial.

/mjt