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

Attachment: signature.asc
Description: OpenPGP digital signature


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