Benoît Canet <benoit.ca...@nodalink.com> writes:
> We will want to reuse this define in the future by making it common to
> multiples
> QEMU modules.
> It would be safer that this define be an integer so we avoid stranges float
> rounding errors.
> Do this conversion.
>
> Signed-off-by: Benoît Canet <benoit.ca...@nodalink.com>
> ---
> include/qemu/throttle.h | 2 +-
> util/throttle.c | 4 ++--
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h
> index b890613..8f9e611 100644
> --- a/include/qemu/throttle.h
> +++ b/include/qemu/throttle.h
> @@ -27,7 +27,7 @@
> #include "qemu-common.h"
> #include "qemu/timer.h"
>
> -#define NANOSECONDS_PER_SECOND 1000000000.0
> +#define NANOSECONDS_PER_SECOND 1000000000
>
> typedef enum {
> THROTTLE_BPS_TOTAL,
> diff --git a/util/throttle.c b/util/throttle.c
> index f976ac7..af8445a 100644
> --- a/util/throttle.c
> +++ b/util/throttle.c
> @@ -34,7 +34,7 @@ void throttle_leak_bucket(LeakyBucket *bkt, int64_t
> delta_ns)
> double leak;
>
> /* compute how much to leak */
> - leak = (bkt->avg * (double) delta_ns) / NANOSECONDS_PER_SECOND;
> + leak = (bkt->avg * (double) delta_ns) / (double) NANOSECONDS_PER_SECOND;
>
> /* make the bucket leak */
> bkt->level = MAX(bkt->level - leak, 0);
> @@ -70,7 +70,7 @@ static void throttle_do_leak(ThrottleState *ts, int64_t now)
> */
> static int64_t throttle_do_compute_wait(double limit, double extra)
> {
> - double wait = extra * NANOSECONDS_PER_SECOND;
> + double wait = extra * (double) NANOSECONDS_PER_SECOND;
> wait /= limit;
> return wait;
> }
I agree with Eric: casts aren't necessary here. Such clutter is okay
when it makes the code easier to understand, or more robust against
someone messing with types. Matter of taste here. I guess I'd omit the
casts.
Other uses of NANOSECONDS_PER_SECOND are in tests/test-throttle.c.
test-throttle.c:38: throttle_leak_bucket(&bkt, NANOSECONDS_PER_SECOND / 150);
test-throttle.c:44: throttle_leak_bucket(&bkt, NANOSECONDS_PER_SECOND / 150);
test-throttle.c:50: throttle_leak_bucket(&bkt, NANOSECONDS_PER_SECOND / 150);
test-throttle.c:88: result = (int64_t) NANOSECONDS_PER_SECOND / 150 / 2;
Your patch makes it divide in int, then widen to int64_t. Before, it
divided in double, then converted to int64_t, discarding fractions.
Looks okay.
