[Qemu-devel] [PATCH] nbd-client: Use correct macro parenthesization

[Qemu-devel] [PATCH] nbd-client: Use correct macro parenthesization

[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 <eblake@redhat.com>
---

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(-)

diff --git a/block/nbd-client.c b/block/nbd-client.c
index 5f241ecc22..2a528dd217 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -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)
 {
-- 
2.13.5

Re: [Qemu-devel] [Qemu-trivial] [PATCH] nbd-client: Use correct macro parenthesization

[Philippe Mathieu-Daudé]

Hi Eric,

On 09/18/2017 06:46 PM, Eric Blake wrote:
> 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 <eblake@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

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

> ---
> 
> 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(-)
> 
> diff --git a/block/nbd-client.c b/block/nbd-client.c
> index 5f241ecc22..2a528dd217 100644
> --- a/block/nbd-client.c
> +++ b/block/nbd-client.c
> @@ -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)
>   {
> 

Re: [Qemu-devel] [Qemu-trivial] [PATCH] nbd-client: Use correct macro parenthesization

[Eric Blake]

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

On 09/18/2017 05:13 PM, Philippe Mathieu-Daudé wrote:
> Hi Eric,
> 
> On 09/18/2017 06:46 PM, Eric Blake wrote:
>> 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 <eblake@redhat.com>
> 
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> 
> 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.


-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


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

Re: [Qemu-devel] [Qemu-trivial] [PATCH] nbd-client: Use correct macro parenthesization

[Philippe Mathieu-Daudé]

On 09/18/2017 07:42 PM, Eric Blake wrote:
> On 09/18/2017 05:13 PM, Philippe Mathieu-Daudé wrote:
>> 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))

Re: [Qemu-devel] [Qemu-trivial] [PATCH] nbd-client: Use correct macro parenthesization

[Eric Blake]

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

On 09/19/2017 05:32 AM, Philippe Mathieu-Daudé wrote:
> On 09/18/2017 07:42 PM, Eric Blake wrote:
>> On 09/18/2017 05:13 PM, Philippe Mathieu-Daudé wrote:
>>> 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))

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

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


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

Re: [Qemu-devel] [Qemu-block] [PATCH] nbd-client: Use correct macro parenthesization

[Stefan Hajnoczi]

On Mon, Sep 18, 2017 at 04:46:49PM -0500, Eric Blake wrote:
> 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 <eblake@redhat.com>
> ---
> 
> 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 <stefanha@redhat.com>

Re: [Qemu-devel] [PATCH] nbd-client: Use correct macro parenthesization

[Michael Tokarev]

19.09.2017 00:46, Eric Blake wrote:
> 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