Re: [Qemu-devel] [PATCH v2 2/3] host-utils: Proactively fix pow2floor(), switch to unsigned

[Eric Blake]

On 07/27/2017 04:46 AM, Markus Armbruster wrote:
> The function's stated contract is simple enough: "round down to the
> nearest power of 2".  Suggests the domain is the representable numbers
>> = 1, because that's the smallest power of two.

Mail quoting gives an interesting rendering of this line; I had to do a
double-take to realize your text is correct.

> 
> Fix by switching from int64_t to uint64_t and amending the contract to
> map zero to zero.

I like it.  And your proof of auditing callers helps.

> ---
>  include/qemu/host-utils.h | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/include/qemu/host-utils.h b/include/qemu/host-utils.h
> index 95cf4f4..6c6005f 100644
> --- a/include/qemu/host-utils.h
> +++ b/include/qemu/host-utils.h
> @@ -369,13 +369,16 @@ static inline bool is_power_of_2(uint64_t value)
>      return !(value & (value - 1));
>  }
>  
> -/* round down to the nearest power of 2*/
> -static inline int64_t pow2floor(int64_t value)
> +/**
> + * Return @value rounded down to the nearest power of two or zero.
> + */
> +static inline uint64_t pow2floor(uint64_t value)
>  {
> -    if (!is_power_of_2(value)) {
> -        value = 0x8000000000000000ULL >> clz64(value);
> +    if (!value) {
> +        /* Avoid undefined shift by 64 */
> +        return 0;
>      }
> -    return value;
> +    return 0x8000000000000000ull >> clz64(value);
>  }

Reviewed-by: Eric Blake <address@hidden>

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

signature.asc
Description: OpenPGP digital signature