Re: [Qemu-devel] [PATCH v3] bitops: unify bitops_ffsl with the one in host-utils.h, call it bitops_ctzl

[Eric Blake]

On 02/01/2013 03:03 PM, Paolo Bonzini wrote:
> We had two copies of a ffs function for longs with subtly different
> semantics and, for the one in bitops.h, a confusing name: the result
> was off-by-one compared to the library function ffsl.
> 
> Unify the functions into one, and solve the name problem by calling
> the 0-based functions "bitops_ctzl" and "bitops_ctol" respectively.
> 
> This also fixes the build on platforms with ffsl, including Mac OS X
> and Windows.
> 
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
>  

>  /**
> - * bitops_ffs - find first bit in word.
> + * bitops_ctzl - count trailing zeroes in word.
>   * @word: The word to search
>   *
> - * Undefined if no bit exists, so code should check against 0 first.
> + * Returns -1 if no bit exists.  Note that compared to the C library
> + * routine ffsl, this one returns one less.
>   */
> -static unsigned long bitops_ffsl(unsigned long word)
> +static unsigned long bitops_ctzl(unsigned long word)

The C library ffsl() returns 'int', not 'unsigned long'.  Given that
your bitops_ctzl returns -1, does it make sense to fix this to return a
signed type?

>  
>  /**
> - * ffz - find first zero in word.
> + * cto - count trailing ones in word.
>   * @word: The word to search
>   *
> - * Undefined if no zero exists, so code should check against ~0UL first.
> + * Returns -1 if all bit are set.
>   */
> -static inline unsigned long ffz(unsigned long word)
> +static inline unsigned long bitops_ctol(unsigned long word)

Likewise; if we are changing this name, should we also change the return
type to be signed?

However, the wrong return type doesn't impact any of the callers, and
this time around, your conversion looks correct.

Reviewed-by: Eric Blake <address@hidden>

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

signature.asc
Description: OpenPGP digital signature