[Libguestfs] [PATCH nbdkit v2] common: Introduce round up, down; and divide round up functions.

[Eric Blake]

On 9/17/18 3:38 PM, Richard W.M. Jones wrote:
> These are used at various places in the code already.  This
> refactoring simply moves them to a common header file and should have
> no other effect.
> ---
>   common/include/rounding.h   | 59 +++++++++++++++++++++++++++++++++++++
>   filters/cache/Makefile.am   |  3 +-
>   filters/cache/cache.c       |  2 +-
>   filters/cow/Makefile.am     |  3 +-
>   filters/cow/cow.c           |  2 +-
>   filters/truncate/truncate.c |  5 ++--
>   6 files changed, 68 insertions(+), 6 deletions(-)
> 

> +/* Round up i to next multiple of n (n must be a power of 2).
> + */
> +#define ROUND_UP(i, n) ({                          \
> +      assert (is_power_of_2 (n));                  \
> +      ((i) + (n) - 1) & ~((n) - 1);                \
> +})

Potential bug: if n is 32-bit unsigned, but i is 64-bit, the bitmask on 
the right half of & will be inappropriately truncated (no sign extension 
is performed).

> +
> +/* Round down i to next multiple of n (n must be a power of 2).
> + */
> +#define ROUND_DOWN(i, n) ({                         \
> +      assert (is_power_of_2 (n));                   \
> +      (i) & ~((n) - 1);                             \
> +})

And again.

Also, ~((n)-1) is identical to -(n), if you want less typing (well, as 
long as you assume twos-complements signed numbers, but that's pretty 
safe to assume these days).

> +
> +/* Return n / d, rounding the result up to the next integer. */
> +#define DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d))

This one should be okay, though.

> +++ b/filters/truncate/truncate.c
> @@ -46,6 +46,7 @@
>   
>   #include "ispowerof2.h"
>   #include "iszero.h"
> +#include "rounding.h"
>   
>   #define THREAD_MODEL NBDKIT_THREAD_MODEL_PARALLEL
>   
> @@ -157,9 +158,9 @@ truncate_get_size (struct nbdkit_next_ops *next_ops, void *nxdata,
>     if (truncate_size >= 0)
>       size = truncate_size;
>     if (round_up > 0)
> -    size = (size + round_up - 1) & ~(round_up - 1);
> +    size = ROUND_UP (size, round_up);
>     if (round_down > 0)
> -    size &= ~(round_down - 1);
> +    size = ROUND_DOWN (size, round_down);

Ouch - the bug was pre-existing.  size is uint64_t, while 
round_{up,down} is unsigned (and hence 32-bit), which means this failed 
on sizes larger than 4G.

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