Re: [PULL 25/31] osdep: Make MIN/MAX evaluate arguments only once

[Eric Blake]

On 6/24/20 5:50 AM, Paolo Bonzini wrote:
> From: Eric Blake <eblake@redhat.com>
>
> I'm not aware of any immediate bugs in qemu where a second runtime
> evalution of the arguments to MIN() or MAX() causes a problem, but

evaluation

> Update the MIN/MAX macros to only evaluate their argument once at
> runtime;

> Use of MIN when MIN_CONST is needed:
>
> In file included from /home/eblake/qemu/qemu-img.c:25:
> /home/eblake/qemu/include/qemu/osdep.h:249:5: error: braced-group within 
> expression allowed only inside a function
>    249 |     ({                                                  \
>        |     ^
> /home/eblake/qemu/qemu-img.c:92:12: note: in expansion of macro ‘MIN’

UTF-8 mojibake in the commit message ;(


> Signed-off-by: Eric Blake <eblake@redhat.com>
>
> Message-Id: <20200604215236.2798244-1-eblake@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---

> +#define MIN_CONST(a, b)                                         \
> +    __builtin_choose_expr(                                      \
> +        __builtin_constant_p(a) && __builtin_constant_p(b),     \
> +        (a) < (b) ? (a) : (b),                                  \
> +        ((void)0))

This one is correct...

> +#undef MAX
> +#define MAX(a, b)                                       \
> +    ({                                                  \
> +        typeof(1 ? (a) : (b)) _a = (a), _b = (b);       \
> +        _a > _b ? _a : _b;                              \
> +    })
> +#define MAX_CONST(a, b)                                         \
> +    __builtin_choose_expr(                                      \
> +        __builtin_constant_p(a) && __builtin_constant_p(b),     \
> +        (a) > (b) ? (a) : (b),                                  \
> +        __builtin_unreachable())

...but this one should also use ((void)0), to match the commit message.

> +
> +/* Minimum function that returns zero only if both values are zero.
>    * Intended for use with unsigned values only. */

And checkpatch complains about the formatting here.

Ah well.  I had all these things fixed in my tree, but failed to post a 
v5.  Not worth holding up this pull request in isolation, but if there 
are any other build issues, I'll post a v5 of this patch, otherwise a 
followup.

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