Re: [PATCH 3/3] qemu-timer: reuse MIN macro in qemu_timeout_ns_to_ms

[Eric Blake <eblake@redhat.com>]

On 10/23/19 9:15 AM, Frediano Ziglio wrote:
>>
>> On 10/23/19 8:42 AM, Laurent Vivier wrote:
>>> Le 23/10/2019 à 14:26, Frediano Ziglio a écrit :
>>>> Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
>>>> ---
>>>>    util/qemu-timer.c | 6 +-----
>>>>    1 file changed, 1 insertion(+), 5 deletions(-)
>>>>
>>>> diff --git a/util/qemu-timer.c b/util/qemu-timer.c
>>>> index d428fec567..094a20a05a 100644
>>>> --- a/util/qemu-timer.c
>>>> +++ b/util/qemu-timer.c
>>>> @@ -322,11 +322,7 @@ int qemu_timeout_ns_to_ms(int64_t ns)
>>>>        ms = DIV_ROUND_UP(ns, SCALE_MS);
>>>>    
>>>>        /* To avoid overflow problems, limit this to 2^31, i.e. approx 25
>>>>        days */
>>>> -    if (ms > (int64_t) INT32_MAX) {
>>>> -        ms = INT32_MAX;
>>>> -    }
>>>> -
>>>> -    return (int) ms;
>>>> +    return (int) MIN(ms, (int64_t) INT32_MAX);
>>>>    }
>>
>> Why so many casts?  It should also work as:
>>
>> return MIN(ms, INT32_MAX);
>>
> 
> This was former version. Laurent pointed out that MIN macro
> is using ternary operator which is expected to find the same time
> on second and third part so the cast inside the MIN macro.
> The cast before MIN was kept from previous code.

The C rules for ternary type promotion guarantee that the MIN macro 
produces the correct type without the cast ('cond ? int64_t : int32_t' 
produces int64_t).  I agree that the (int) cast was pre-existing, but as 
long as we are cleaning up useless casts, we might as well clean up both.

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