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

[Peter Maydell]

On 1 February 2013 22:03, Paolo Bonzini <address@hidden> 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>

Tested-by: Peter Maydell <address@hidden>

This fixes the MacOSX build failure and boots a guest OK.

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

Do any of our callers actually use the "-1 on 0 input" semantics?
(I guess that means "your new code you added" since the previous
callers all were happy with the undefined-on-zero semantics).
It seems an odd choice, since I can see a justification for:
 (a) "return number of bits in word" [every bit in the word is
     a trailing zero in some sense]
 (b) "undefined" [matches gcc builtin_ctz semantics]

However I'm more interested in getting a reasonable patch
for the build issues committed before the next rc rather
than spending too much time nitpicking details.

-- PMM