[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH V2 1/3] Add function to find and set next available bit



On 02/01/2013 02:57 PM, Stefan Berger wrote:
> Add function to find the next available bit in the bitmap and set it.
> 
> Signed-off-by: Stefan Berger <stefanb linux vnet ibm com>
> 
> ---
>  src/libvirt_private.syms |    1 +
>  src/util/virbitmap.c     |   34 ++++++++++++++++++++++++++++++++++
>  src/util/virbitmap.h     |    3 +++
>  3 files changed, 38 insertions(+)
> 
> Index: libvirt/src/util/virbitmap.c
> ===================================================================
> --- libvirt.orig/src/util/virbitmap.c
> +++ libvirt/src/util/virbitmap.c
> @@ -642,3 +642,37 @@ virBitmapCountBits(virBitmapPtr bitmap)
>  
>      return ret;
>  }

> @@ -106,4 +106,7 @@ ssize_t virBitmapNextSetBit(virBitmapPtr
>  size_t virBitmapCountBits(virBitmapPtr bitmap)
>      ATTRIBUTE_NONNULL(1);
>  
> +size_t virBitmapSetNextBit(virBitmapPtr bitmap, ssize_t pos)
> +    ATTRIBUTE_NONNULL(1);

With this patch, we'd have:

virBitmapNextSetBit() - find without modifying
virBitmapSetNextBit() - find and modify

that feels a bit confusing (without reading the description, which one
do you want?).  Also, your implementation is slow (testing one bit at a
time is slower than using tricks like ffsl).  I'd rather see a
virBitmapNextClearBit() with same semantics of virBitmapNextSetBit at
iterating through the bitmap using ffsl() optimizations, then have
callers explicitly set the returned bit, instead of providing a
find-and-set function.

And our testsuite should be updated to cover this new function.

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

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]