Re: [Qemu-devel] [PATCH v2 1/3] throttle: Make NANOSECONDS_PER_SECOND an integer

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2289 bytes --]

On 09/08/2014 06:18 AM, Benoît Canet wrote:
> We will want to reuse this define in the future by making it common to multiples

s/multiples/multiple/

> QEMU modules.
> It would be safer that this define be an integer so we avoid stranges float

s/stranges/strange/

> rounding errors.
> Do this conversion.
> 
> Signed-off-by: Benoît Canet <benoit.canet@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

This hunk is good.

>  
>  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;

This hunk is spurious.  With just your first hunk, it evaluates to the
following types via promotion rules:

(double * double) / int
double / int
double / double
double

so the explicit cast isn't changing anything.

>  
>      /* 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;

This hunk is also spurious.  Again, with just your first hunk, it
evaluates through the following promotion rules:

double * int
double * double
double

and the cast isn't changing anything.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]