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

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

 



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@xxxxxxxxxx    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]