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

[Eric Blake]

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 <address@hidden>
> > > > ---
> > > >    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