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

Re: [libvirt] [PATCHv3 1/4] bitmap: add virBitmapCountBits



On 10/25/2012 08:35 AM, Viktor Mihajlovski wrote:
> On 10/25/2012 03:36 AM, Eric Blake wrote:
>> Sometimes it's handy to know how many bits are set.
>>
>> * src/util/bitmap.h (virBitmapCountBits): New prototype.
>> (virBitmapNextSetBit): Use correct type.
>> * src/util/bitmap.c (virBitmapNextSetBit): Likewise.
>> (virBitmapCountBits): New function.
>> * src/libvirt_private.syms (bitmap.h): Export it.
>> * tests/virbitmaptest.c (test2): Test it.
> You might want to add a sign-off.
>> +/* Return the number of bits currently set in the map.  */
>> +size_t
>> +virBitmapCountBits(virBitmapPtr bitmap)
>> +{
>> +    size_t i;
>> +    size_t ret = 0;
>> +    int tail = bitmap->max_bit % VIR_BITMAP_BITS_PER_UNIT;
>> +
>> +    /* Ensure tail bits are clear.  */
>> +    if (tail)
>> +        bitmap->map[bitmap->map_len - 1] &=
>> +            -1UL >> (VIR_BITMAP_BITS_PER_UNIT - tail);
> Probably not necessary, as the bitmap is initialized to zero.

Absolutely necessary, or 'make check' fails.  But debatable whether to
put it here, or to shift it to virBitmapSetAll() (as THAT appears to be
the only culprit function that ever sets tail-bits to 1).

> 
> Works for me.

Thanks for the review.  I think I'll move the tail clearing to
virBitmapSetAll, as that is likely to be the less-frequently called
function, and maintaining the invariant that tail bits are always clear
seems nicer than assuming they are undefined and having to explicitly
clear tail bits prior to a count operation.

-- 
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]