Re: [libvirt] [PATCHv3 1/4] bitmap: add virBitmapCountBits
On 10/25/2012 10:46 AM, Eric Blake wrote:
>> +
>> + /* 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.
I've now pushed the series with this squashed in. Thanks for your
initial work, and for testing my changes.
diff --git i/src/util/bitmap.c w/src/util/bitmap.c
index 9a9152a..2dd3403 100644
--- i/src/util/bitmap.c
+++ w/src/util/bitmap.c
@@ -528,8 +528,15 @@ size_t virBitmapSize(virBitmapPtr bitmap)
*/
void virBitmapSetAll(virBitmapPtr bitmap)
{
+ int tail = bitmap->max_bit % VIR_BITMAP_BITS_PER_UNIT;
+
memset(bitmap->map, 0xff,
bitmap->map_len * (VIR_BITMAP_BITS_PER_UNIT / CHAR_BIT));
+
+ /* Ensure tail bits are clear. */
+ if (tail)
+ bitmap->map[bitmap->map_len - 1] &=
+ -1UL >> (VIR_BITMAP_BITS_PER_UNIT - tail);
}
/**
@@ -621,12 +628,6 @@ 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);
for (i = 0; i < bitmap->map_len; i++)
ret += count_one_bits_l(bitmap->map[i]);
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 617 bytes)