[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 3/3] qemu-timer: reuse MIN macro in qemu_timeout_ns_to_ms
From: |
Frediano Ziglio |
Subject: |
Re: [PATCH 3/3] qemu-timer: reuse MIN macro in qemu_timeout_ns_to_ms |
Date: |
Fri, 27 Sep 2019 07:57:05 -0400 (EDT) |
>
> Le 27/09/2019 à 12:32, 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..9bd173ecda 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, INT32_MAX);
> > }
> >
>
> Perhaps it would be cleaner to have always the same return type for
> MIN() with:
>
> MIN(ms, (int64_t) INT32_MAX)
>
The (int64_t) cast here is useless, ms is signed, C compilers knows how to
convert and compare properly.
The function returns int and it's used in some cases to fill 32bit integer,
changing the return type to int64_t is not trivial.
But I suppose that here you mean instead that the 2 sides of the ternary
operator used by MIN macro are int64_t and (probably) int32_t. In this case
feel free to add the cast with a
return (int) MIN(ms, (int64_t) INT32_MAX);
> Anyway:
>
> Reviewed-by: Laurent Vivier <address@hidden>
>
> Thanks,
> Laurent
>
Frediano