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