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 [email protected] +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list [email protected] https://www.redhat.com/mailman/listinfo/libvir-list
