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

Re: [libvirt] [PATCH] bitmap: add way to find next clear bit



On 02/01/2013 09:41 PM, Stefan Berger wrote:
> On 02/01/2013 09:16 PM, Eric Blake wrote:
>> We had an easy way to iterate set bits, but not for iterating
>> cleared bits.
>>
>> * src/util/virbitmap.h (virBitmapNextClearBit): New prototype.
>> * src/util/virbitmap.c (virBitmapNextClearBit): Implement it.
>> * src/libvirt_private.syms (bitmap.h): Export it.
>> * tests/virbitmaptest.c (test4): Test it.
>> ---
> 
> 
> I didn't test it yours so far... Here's at least the part of the test I
> had fabricated (your bitmap extension seems better):
> 
> Index: libvirt/tests/virbitmaptest.c
> ===================================================================
> --- libvirt.orig/tests/virbitmaptest.c
> +++ libvirt/tests/virbitmaptest.c
> @@ -390,6 +390,74 @@ error:
>      return -1;
>  }
> 
> +/* test for virBitmapNextClearBit */
> +static int test8(const void *data ATTRIBUTE_UNUSED)
> +{
> +    const char *bitsString = "0, 2-4, 6-10, 12, 14-18, 20, 22, 25,
> 28-33, 35, "
> +        "41-42, 45-46";
> +    int size = 48;
> +    int noBitsPos[] = {
> +        1,  5,  11, 13, 19, 21, 23, 24,
> +        26, 27, 34, 36, 37, 38, 39, 40,
> +        43, 44, 47
> +    };
> +    int npos = 19;

I definitely prefer the ARRAY_CARDINALITY() instead of npos in my
version of the test.  Beyond that, it looks like your test copied and
pasted so that the set vs. clear tests were independent, while mine
interleaved the two to traverse over the same set.  Coverage wise,
either one of our tests gets just as much coverage of the added code; at
least, I didn't see anything added by your test8() that wasn't present
in my revised test4().

> 
> Maybe we can merge the two for a joint effort :-)  -- leaving test 4
> untouched.

Anyone else have an opinion?

> 
> Patch 3 of the series has calls missing regarding clearing of the bitmap
> upon failure and VM shutdown. I added that after sending.
> 
> Regards,
>    Stefan
> 
> 
> 

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