[PATCH] libxl: provide libxl_bitmap_{and,or}

[PATCH] libxl: provide libxl_bitmap_{and,or}

[Linda Jacobson]

Added functions to create the logical 'and' and logical 'or'
of two input bitmaps.  Cleaned up spacing and comments.

Removed accidentally committed libxl_u_disk* files.

Signed-off-by: Linda Jacobson <lindaj@jma3.com>
---
 tools/libxl/libxl_utils.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++
 tools/libxl/libxl_utils.h |  6 ++++++
 2 files changed, 61 insertions(+)

diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index 9053b27..fb8830a 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -691,6 +691,61 @@ void libxl_bitmap_reset(libxl_bitmap *bitmap, int bit)
     bitmap->map[bit / 8] &= ~(1 << (bit & 7));
 }
 
+int libxl_bitmap_or(libxl_ctx *ctx, libxl_bitmap *or_map,
+                    libxl_bitmap *map1, libxl_bitmap *map2)
+{
+    uint32_t size;
+    int rc;
+    int bit;
+
+    GC_INIT(ctx);
+    /* if bitmaps aren't same size, the bitmap of their logical 
+       or should be the size of the larger bit map
+    */
+    size = max(map1->size, map2->size);
+    rc = libxl_bitmap_alloc(ctx, or_map, size);
+    if (rc)
+        goto out;
+
+    for (bit = 0; bit < (size * 8); bit++) {
+        if (libxl_bitmap_test(map1, bit))
+            libxl_bitmap_set(or_map, bit);
+        else if (libxl_bitmap_test(map2, bit))
+            libxl_bitmap_set(or_map, bit);
+    }
+
+out:
+    GC_FREE;
+    return rc;
+} 
+
+int libxl_bitmap_and(libxl_ctx *ctx, libxl_bitmap *and_map,
+                     libxl_bitmap *map1, libxl_bitmap *map2)
+{
+    uint32_t size;
+    int rc;
+    int bit;
+
+    GC_INIT(ctx);
+    /* if bitmaps aren't same size, logical and should
+       be the size of the smaller bit map
+    */ 
+    size = min(map1->size, map2->size);
+    rc = libxl_bitmap_alloc(ctx, and_map, size);
+    if (rc)
+        goto out;
+
+    for (bit = 0; bit < (size * 8); bit++) {
+        if (libxl_bitmap_test (map1, bit) && 
+            libxl_bitmap_test (map2, bit) )
+            libxl_bitmap_set (and_map, bit);
+    }
+
+out:
+    GC_FREE;
+    return rc;
+ } 
+
 int libxl_bitmap_count_set(const libxl_bitmap *bitmap)
 {
     int i, nr_set_bits = 0;
diff --git a/tools/libxl/libxl_utils.h b/tools/libxl/libxl_utils.h
index 68b5580..0b6480d 100644
--- a/tools/libxl/libxl_utils.h
+++ b/tools/libxl/libxl_utils.h
@@ -91,6 +91,12 @@ void libxl_bitmap_set(libxl_bitmap *bitmap, int bit);
 void libxl_bitmap_reset(libxl_bitmap *bitmap, int bit);
 int libxl_bitmap_count_set(const libxl_bitmap *bitmap);
 char *libxl_bitmap_to_hex_string(libxl_ctx *ctx, const libxl_bitmap *bitmap);
+/* or, and and xor functions for two bitmaps
+ */
+int libxl_bitmap_or(libxl_ctx *ctx, libxl_bitmap *or_map,
+                    libxl_bitmap *map1, libxl_bitmap *map2); 
+int libxl_bitmap_and(libxl_ctx *ctx, libxl_bitmap *and_map,
+                     libxl_bitmap *map1, libxl_bitmap *map2);
 static inline void libxl_bitmap_set_any(libxl_bitmap *bitmap)
 {
     memset(bitmap->map, -1, bitmap->size);
-- 
1.9.1

Re: [PATCH] libxl: provide libxl_bitmap_{and,or}

[Julien Grall]

Hi Linda,

Thank you for sending the new version.

It's common to CC the maintainers of the patch. Most of us have filter 
in order to get directly mail they are involved too and avoid watching 
every time the ML. Otherwise answer to your mail may take longer.

You can find them with the scripts scripts/get_maintainers.pl. I've cced 
them.

On 10/04/2015 04:34, Linda Jacobson wrote:
> Added functions to create the logical 'and' and logical 'or'
> of two input bitmaps.  Cleaned up spacing and comments.
>
> Removed accidentally committed libxl_u_disk* files.

Thank you for saying what you changed in this version. While it's useful 
during review process, this is not necessary to keep them in the final 
commit message.

When the committers will commit your patch, git will ignore everything 
after "---" and a newline.

For instance you could write

---
    Changes in v3:
	- Foo ...

    Changes in v2:
	- Added functions...
         - Removed ...


> Signed-off-by: Linda Jacobson <lindaj@jma3.com>
> ---
>   tools/libxl/libxl_utils.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++
>   tools/libxl/libxl_utils.h |  6 ++++++
>   2 files changed, 61 insertions(+)
>
> diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
> index 9053b27..fb8830a 100644
> --- a/tools/libxl/libxl_utils.c
> +++ b/tools/libxl/libxl_utils.c
> @@ -691,6 +691,61 @@ void libxl_bitmap_reset(libxl_bitmap *bitmap, int bit)
>       bitmap->map[bit / 8] &= ~(1 << (bit & 7));
>   }
>
> +int libxl_bitmap_or(libxl_ctx *ctx, libxl_bitmap *or_map,
> +                    libxl_bitmap *map1, libxl_bitmap *map2)
> +{
> +    uint32_t size;
> +    int rc;
> +    int bit;
> +
> +    GC_INIT(ctx);

GC_INIT is a macro which declaring a variable. It seems common to 
declare at the very beginning of the function. Can you move it there?

> +    /* if bitmaps aren't same size, the bitmap of their logical
> +       or should be the size of the larger bit map
> +    */

The coding style for multi-lines comment is:

/*
  * My comment
  * continue
  */

> +    size = max(map1->size, map2->size);
> +    rc = libxl_bitmap_alloc(ctx, or_map, size);
> +    if (rc)
> +        goto out;
> +
> +    for (bit = 0; bit < (size * 8); bit++) {
> +        if (libxl_bitmap_test(map1, bit))
> +            libxl_bitmap_set(or_map, bit);
> +        else if (libxl_bitmap_test(map2, bit))
> +            libxl_bitmap_set(or_map, bit);
> +    }
> +
> +out:
> +    GC_FREE;
> +    return rc;
> +}
> +
> +int libxl_bitmap_and(libxl_ctx *ctx, libxl_bitmap *and_map,
> +                     libxl_bitmap *map1, libxl_bitmap *map2)
> +{
> +    uint32_t size;
> +    int rc;
> +    int bit;
> +
> +    GC_INIT(ctx);

Same remark.

> +    /* if bitmaps aren't same size, logical and should
> +       be the size of the smaller bit map
> +    */

Coding style

> +    size = min(map1->size, map2->size);
> +    rc = libxl_bitmap_alloc(ctx, and_map, size);
> +    if (rc)
> +        goto out;
> +
> +    for (bit = 0; bit < (size * 8); bit++) {
> +        if (libxl_bitmap_test (map1, bit) &&
> +            libxl_bitmap_test (map2, bit) )
> +            libxl_bitmap_set (and_map, bit);
> +    }
> +
> +out:
> +    GC_FREE;
> +    return rc;
> + }
> +
>   int libxl_bitmap_count_set(const libxl_bitmap *bitmap)
>   {
>       int i, nr_set_bits = 0;
> diff --git a/tools/libxl/libxl_utils.h b/tools/libxl/libxl_utils.h
> index 68b5580..0b6480d 100644
> --- a/tools/libxl/libxl_utils.h
> +++ b/tools/libxl/libxl_utils.h
> @@ -91,6 +91,12 @@ void libxl_bitmap_set(libxl_bitmap *bitmap, int bit);
>   void libxl_bitmap_reset(libxl_bitmap *bitmap, int bit);
>   int libxl_bitmap_count_set(const libxl_bitmap *bitmap);
>   char *libxl_bitmap_to_hex_string(libxl_ctx *ctx, const libxl_bitmap *bitmap);
> +/* or, and and xor functions for two bitmaps
> + */

I saw you sent a separate patch for fixing the coding style. I don't 
think it's necessary. Can you merge the 2 patches?

Although, the coding style for a one line comment is
/* My comment */

> +int libxl_bitmap_or(libxl_ctx *ctx, libxl_bitmap *or_map,
> +                    libxl_bitmap *map1, libxl_bitmap *map2);
> +int libxl_bitmap_and(libxl_ctx *ctx, libxl_bitmap *and_map,
> +                     libxl_bitmap *map1, libxl_bitmap *map2);
>   static inline void libxl_bitmap_set_any(libxl_bitmap *bitmap)
>   {
>       memset(bitmap->map, -1, bitmap->size);
>

Other than few coding style issue, I think the 2 functions (or & and) 
can be optimize (i.e working with byte rather than bit). Although, I'm 
not a maintainers so I will let them decide if it's worth to do it.

Regards,

-- 
Julien Grall

Re: [PATCH] libxl: provide libxl_bitmap_{and,or}

[Linda]

PS - it's not much work to change to using bytes instead of bits.  I 
agree it would be better.  I'll change it if the maintainers want.

Linda

On 4/11/2015 2:31 PM, Julien Grall wrote:
> Hi Linda,
>
> Thank you for sending the new version.
>
> It's common to CC the maintainers of the patch. Most of us have filter 
> in order to get directly mail they are involved too and avoid watching 
> every time the ML. Otherwise answer to your mail may take longer.
>
> You can find them with the scripts scripts/get_maintainers.pl. I've 
> cced them.
>
> On 10/04/2015 04:34, Linda Jacobson wrote:
>> Added functions to create the logical 'and' and logical 'or'
>> of two input bitmaps.  Cleaned up spacing and comments.
>>
>> Removed accidentally committed libxl_u_disk* files.
>
> Thank you for saying what you changed in this version. While it's 
> useful during review process, this is not necessary to keep them in 
> the final commit message.
>
> When the committers will commit your patch, git will ignore everything 
> after "---" and a newline.
>
> For instance you could write
>
> ---
>    Changes in v3:
>     - Foo ...
>
>    Changes in v2:
>     - Added functions...
>         - Removed ...
>
>
>> Signed-off-by: Linda Jacobson <lindaj@jma3.com>
>> ---
>>   tools/libxl/libxl_utils.c | 55 
>> +++++++++++++++++++++++++++++++++++++++++++++++
>>   tools/libxl/libxl_utils.h |  6 ++++++
>>   2 files changed, 61 insertions(+)
>>
>> diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
>> index 9053b27..fb8830a 100644
>> --- a/tools/libxl/libxl_utils.c
>> +++ b/tools/libxl/libxl_utils.c
>> @@ -691,6 +691,61 @@ void libxl_bitmap_reset(libxl_bitmap *bitmap, 
>> int bit)
>>       bitmap->map[bit / 8] &= ~(1 << (bit & 7));
>>   }
>>
>> +int libxl_bitmap_or(libxl_ctx *ctx, libxl_bitmap *or_map,
>> +                    libxl_bitmap *map1, libxl_bitmap *map2)
>> +{
>> +    uint32_t size;
>> +    int rc;
>> +    int bit;
>> +
>> +    GC_INIT(ctx);
>
> GC_INIT is a macro which declaring a variable. It seems common to 
> declare at the very beginning of the function. Can you move it there?
>
>> +    /* if bitmaps aren't same size, the bitmap of their logical
>> +       or should be the size of the larger bit map
>> +    */
>
> The coding style for multi-lines comment is:
>
> /*
>  * My comment
>  * continue
>  */
>
>> +    size = max(map1->size, map2->size);
>> +    rc = libxl_bitmap_alloc(ctx, or_map, size);
>> +    if (rc)
>> +        goto out;
>> +
>> +    for (bit = 0; bit < (size * 8); bit++) {
>> +        if (libxl_bitmap_test(map1, bit))
>> +            libxl_bitmap_set(or_map, bit);
>> +        else if (libxl_bitmap_test(map2, bit))
>> +            libxl_bitmap_set(or_map, bit);
>> +    }
>> +
>> +out:
>> +    GC_FREE;
>> +    return rc;
>> +}
>> +
>> +int libxl_bitmap_and(libxl_ctx *ctx, libxl_bitmap *and_map,
>> +                     libxl_bitmap *map1, libxl_bitmap *map2)
>> +{
>> +    uint32_t size;
>> +    int rc;
>> +    int bit;
>> +
>> +    GC_INIT(ctx);
>
> Same remark.
>
>> +    /* if bitmaps aren't same size, logical and should
>> +       be the size of the smaller bit map
>> +    */
>
> Coding style
>
>> +    size = min(map1->size, map2->size);
>> +    rc = libxl_bitmap_alloc(ctx, and_map, size);
>> +    if (rc)
>> +        goto out;
>> +
>> +    for (bit = 0; bit < (size * 8); bit++) {
>> +        if (libxl_bitmap_test (map1, bit) &&
>> +            libxl_bitmap_test (map2, bit) )
>> +            libxl_bitmap_set (and_map, bit);
>> +    }
>> +
>> +out:
>> +    GC_FREE;
>> +    return rc;
>> + }
>> +
>>   int libxl_bitmap_count_set(const libxl_bitmap *bitmap)
>>   {
>>       int i, nr_set_bits = 0;
>> diff --git a/tools/libxl/libxl_utils.h b/tools/libxl/libxl_utils.h
>> index 68b5580..0b6480d 100644
>> --- a/tools/libxl/libxl_utils.h
>> +++ b/tools/libxl/libxl_utils.h
>> @@ -91,6 +91,12 @@ void libxl_bitmap_set(libxl_bitmap *bitmap, int bit);
>>   void libxl_bitmap_reset(libxl_bitmap *bitmap, int bit);
>>   int libxl_bitmap_count_set(const libxl_bitmap *bitmap);
>>   char *libxl_bitmap_to_hex_string(libxl_ctx *ctx, const libxl_bitmap 
>> *bitmap);
>> +/* or, and and xor functions for two bitmaps
>> + */
>
> I saw you sent a separate patch for fixing the coding style. I don't 
> think it's necessary. Can you merge the 2 patches?
>
> Although, the coding style for a one line comment is
> /* My comment */
>
>> +int libxl_bitmap_or(libxl_ctx *ctx, libxl_bitmap *or_map,
>> +                    libxl_bitmap *map1, libxl_bitmap *map2);
>> +int libxl_bitmap_and(libxl_ctx *ctx, libxl_bitmap *and_map,
>> +                     libxl_bitmap *map1, libxl_bitmap *map2);
>>   static inline void libxl_bitmap_set_any(libxl_bitmap *bitmap)
>>   {
>>       memset(bitmap->map, -1, bitmap->size);
>>
>
> Other than few coding style issue, I think the 2 functions (or & and) 
> can be optimize (i.e working with byte rather than bit). Although, I'm 
> not a maintainers so I will let them decide if it's worth to do it.
>
> Regards,
>

Re: [PATCH] libxl: provide libxl_bitmap_{and,or}

[Linda]

Hi Julien and all,
     Julien, re: your comment on moving GC_INIT to the top.
     In Wei's template for the functions, he wrote:
     int rc;
     GC_INIT...
/*    your own stuff */
     GC_FREE;
     return rc;

I can move the other declarations below GC_INIT, but should I leave the 
"int rc;" before it?

Thanks.

Linda


On 4/11/2015 2:31 PM, Julien Grall wrote:
> Hi Linda,
>
> Thank you for sending the new version.
>
> It's common to CC the maintainers of the patch. Most of us have filter 
> in order to get directly mail they are involved too and avoid watching 
> every time the ML. Otherwise answer to your mail may take longer.
>
> You can find them with the scripts scripts/get_maintainers.pl. I've 
> cced them.
>
> On 10/04/2015 04:34, Linda Jacobson wrote:
>> Added functions to create the logical 'and' and logical 'or'
>> of two input bitmaps.  Cleaned up spacing and comments.
>>
>> Removed accidentally committed libxl_u_disk* files.
>
> Thank you for saying what you changed in this version. While it's 
> useful during review process, this is not necessary to keep them in 
> the final commit message.
>
> When the committers will commit your patch, git will ignore everything 
> after "---" and a newline.
>
> For instance you could write
>
> ---
>    Changes in v3:
>     - Foo ...
>
>    Changes in v2:
>     - Added functions...
>         - Removed ...
>
>
>> Signed-off-by: Linda Jacobson <lindaj@jma3.com>
>> ---
>>   tools/libxl/libxl_utils.c | 55 
>> +++++++++++++++++++++++++++++++++++++++++++++++
>>   tools/libxl/libxl_utils.h |  6 ++++++
>>   2 files changed, 61 insertions(+)
>>
>> diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
>> index 9053b27..fb8830a 100644
>> --- a/tools/libxl/libxl_utils.c
>> +++ b/tools/libxl/libxl_utils.c
>> @@ -691,6 +691,61 @@ void libxl_bitmap_reset(libxl_bitmap *bitmap, 
>> int bit)
>>       bitmap->map[bit / 8] &= ~(1 << (bit & 7));
>>   }
>>
>> +int libxl_bitmap_or(libxl_ctx *ctx, libxl_bitmap *or_map,
>> +                    libxl_bitmap *map1, libxl_bitmap *map2)
>> +{
>> +    uint32_t size;
>> +    int rc;
>> +    int bit;
>> +
>> +    GC_INIT(ctx);
>
> GC_INIT is a macro which declaring a variable. It seems common to 
> declare at the very beginning of the function. Can you move it there?
>
>> +    /* if bitmaps aren't same size, the bitmap of their logical
>> +       or should be the size of the larger bit map
>> +    */
>
> The coding style for multi-lines comment is:
>
> /*
>  * My comment
>  * continue
>  */
>
>> +    size = max(map1->size, map2->size);
>> +    rc = libxl_bitmap_alloc(ctx, or_map, size);
>> +    if (rc)
>> +        goto out;
>> +
>> +    for (bit = 0; bit < (size * 8); bit++) {
>> +        if (libxl_bitmap_test(map1, bit))
>> +            libxl_bitmap_set(or_map, bit);
>> +        else if (libxl_bitmap_test(map2, bit))
>> +            libxl_bitmap_set(or_map, bit);
>> +    }
>> +
>> +out:
>> +    GC_FREE;
>> +    return rc;
>> +}
>> +
>> +int libxl_bitmap_and(libxl_ctx *ctx, libxl_bitmap *and_map,
>> +                     libxl_bitmap *map1, libxl_bitmap *map2)
>> +{
>> +    uint32_t size;
>> +    int rc;
>> +    int bit;
>> +
>> +    GC_INIT(ctx);
>
> Same remark.
>
>> +    /* if bitmaps aren't same size, logical and should
>> +       be the size of the smaller bit map
>> +    */
>
> Coding style
>
>> +    size = min(map1->size, map2->size);
>> +    rc = libxl_bitmap_alloc(ctx, and_map, size);
>> +    if (rc)
>> +        goto out;
>> +
>> +    for (bit = 0; bit < (size * 8); bit++) {
>> +        if (libxl_bitmap_test (map1, bit) &&
>> +            libxl_bitmap_test (map2, bit) )
>> +            libxl_bitmap_set (and_map, bit);
>> +    }
>> +
>> +out:
>> +    GC_FREE;
>> +    return rc;
>> + }
>> +
>>   int libxl_bitmap_count_set(const libxl_bitmap *bitmap)
>>   {
>>       int i, nr_set_bits = 0;
>> diff --git a/tools/libxl/libxl_utils.h b/tools/libxl/libxl_utils.h
>> index 68b5580..0b6480d 100644
>> --- a/tools/libxl/libxl_utils.h
>> +++ b/tools/libxl/libxl_utils.h
>> @@ -91,6 +91,12 @@ void libxl_bitmap_set(libxl_bitmap *bitmap, int bit);
>>   void libxl_bitmap_reset(libxl_bitmap *bitmap, int bit);
>>   int libxl_bitmap_count_set(const libxl_bitmap *bitmap);
>>   char *libxl_bitmap_to_hex_string(libxl_ctx *ctx, const libxl_bitmap 
>> *bitmap);
>> +/* or, and and xor functions for two bitmaps
>> + */
>
> I saw you sent a separate patch for fixing the coding style. I don't 
> think it's necessary. Can you merge the 2 patches?
>
> Although, the coding style for a one line comment is
> /* My comment */
>
>> +int libxl_bitmap_or(libxl_ctx *ctx, libxl_bitmap *or_map,
>> +                    libxl_bitmap *map1, libxl_bitmap *map2);
>> +int libxl_bitmap_and(libxl_ctx *ctx, libxl_bitmap *and_map,
>> +                     libxl_bitmap *map1, libxl_bitmap *map2);
>>   static inline void libxl_bitmap_set_any(libxl_bitmap *bitmap)
>>   {
>>       memset(bitmap->map, -1, bitmap->size);
>>
>
> Other than few coding style issue, I think the 2 functions (or & and) 
> can be optimize (i.e working with byte rather than bit). Although, I'm 
> not a maintainers so I will let them decide if it's worth to do it.
>
> Regards,
>

Re: [PATCH] libxl: provide libxl_bitmap_{and,or}

[Linda]

Hi all,
     Just for grins, I created and unit tested a little code snippet for 
'and' and 'or', using bytes instead of bits.  It didn't take that long.  
If you want me to replace the code I submitted with this new code, I'd 
be happy to.  Otherwise, I'll just make the style changes Julian 
requested, (unless there is a disagreement on where the GC_INIT should 
be), and merge the changes.

Linda

On 4/11/2015 2:31 PM, Julien Grall wrote:
> Hi Linda,
>
> Thank you for sending the new version.
>
> It's common to CC the maintainers of the patch. Most of us have filter 
> in order to get directly mail they are involved too and avoid watching 
> every time the ML. Otherwise answer to your mail may take longer.
>
> You can find them with the scripts scripts/get_maintainers.pl. I've 
> cced them.
>
> On 10/04/2015 04:34, Linda Jacobson wrote:
>> Added functions to create the logical 'and' and logical 'or'
>> of two input bitmaps.  Cleaned up spacing and comments.
>>
>> Removed accidentally committed libxl_u_disk* files.
>
> Thank you for saying what you changed in this version. While it's 
> useful during review process, this is not necessary to keep them in 
> the final commit message.
>
> When the committers will commit your patch, git will ignore everything 
> after "---" and a newline.
>
> For instance you could write
>
> ---
>    Changes in v3:
>     - Foo ...
>
>    Changes in v2:
>     - Added functions...
>         - Removed ...
>
>
>> Signed-off-by: Linda Jacobson <lindaj@jma3.com>
>> ---
>>   tools/libxl/libxl_utils.c | 55 
>> +++++++++++++++++++++++++++++++++++++++++++++++
>>   tools/libxl/libxl_utils.h |  6 ++++++
>>   2 files changed, 61 insertions(+)
>>
>> diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
>> index 9053b27..fb8830a 100644
>> --- a/tools/libxl/libxl_utils.c
>> +++ b/tools/libxl/libxl_utils.c
>> @@ -691,6 +691,61 @@ void libxl_bitmap_reset(libxl_bitmap *bitmap, 
>> int bit)
>>       bitmap->map[bit / 8] &= ~(1 << (bit & 7));
>>   }
>>
>> +int libxl_bitmap_or(libxl_ctx *ctx, libxl_bitmap *or_map,
>> +                    libxl_bitmap *map1, libxl_bitmap *map2)
>> +{
>> +    uint32_t size;
>> +    int rc;
>> +    int bit;
>> +
>> +    GC_INIT(ctx);
>
> GC_INIT is a macro which declaring a variable. It seems common to 
> declare at the very beginning of the function. Can you move it there?
>
>> +    /* if bitmaps aren't same size, the bitmap of their logical
>> +       or should be the size of the larger bit map
>> +    */
>
> The coding style for multi-lines comment is:
>
> /*
>  * My comment
>  * continue
>  */
>
>> +    size = max(map1->size, map2->size);
>> +    rc = libxl_bitmap_alloc(ctx, or_map, size);
>> +    if (rc)
>> +        goto out;
>> +
>> +    for (bit = 0; bit < (size * 8); bit++) {
>> +        if (libxl_bitmap_test(map1, bit))
>> +            libxl_bitmap_set(or_map, bit);
>> +        else if (libxl_bitmap_test(map2, bit))
>> +            libxl_bitmap_set(or_map, bit);
>> +    }
>> +
>> +out:
>> +    GC_FREE;
>> +    return rc;
>> +}
>> +
>> +int libxl_bitmap_and(libxl_ctx *ctx, libxl_bitmap *and_map,
>> +                     libxl_bitmap *map1, libxl_bitmap *map2)
>> +{
>> +    uint32_t size;
>> +    int rc;
>> +    int bit;
>> +
>> +    GC_INIT(ctx);
>
> Same remark.
>
>> +    /* if bitmaps aren't same size, logical and should
>> +       be the size of the smaller bit map
>> +    */
>
> Coding style
>
>> +    size = min(map1->size, map2->size);
>> +    rc = libxl_bitmap_alloc(ctx, and_map, size);
>> +    if (rc)
>> +        goto out;
>> +
>> +    for (bit = 0; bit < (size * 8); bit++) {
>> +        if (libxl_bitmap_test (map1, bit) &&
>> +            libxl_bitmap_test (map2, bit) )
>> +            libxl_bitmap_set (and_map, bit);
>> +    }
>> +
>> +out:
>> +    GC_FREE;
>> +    return rc;
>> + }
>> +
>>   int libxl_bitmap_count_set(const libxl_bitmap *bitmap)
>>   {
>>       int i, nr_set_bits = 0;
>> diff --git a/tools/libxl/libxl_utils.h b/tools/libxl/libxl_utils.h
>> index 68b5580..0b6480d 100644
>> --- a/tools/libxl/libxl_utils.h
>> +++ b/tools/libxl/libxl_utils.h
>> @@ -91,6 +91,12 @@ void libxl_bitmap_set(libxl_bitmap *bitmap, int bit);
>>   void libxl_bitmap_reset(libxl_bitmap *bitmap, int bit);
>>   int libxl_bitmap_count_set(const libxl_bitmap *bitmap);
>>   char *libxl_bitmap_to_hex_string(libxl_ctx *ctx, const libxl_bitmap 
>> *bitmap);
>> +/* or, and and xor functions for two bitmaps
>> + */
>
> I saw you sent a separate patch for fixing the coding style. I don't 
> think it's necessary. Can you merge the 2 patches?
>
> Although, the coding style for a one line comment is
> /* My comment */
>
>> +int libxl_bitmap_or(libxl_ctx *ctx, libxl_bitmap *or_map,
>> +                    libxl_bitmap *map1, libxl_bitmap *map2);
>> +int libxl_bitmap_and(libxl_ctx *ctx, libxl_bitmap *and_map,
>> +                     libxl_bitmap *map1, libxl_bitmap *map2);
>>   static inline void libxl_bitmap_set_any(libxl_bitmap *bitmap)
>>   {
>>       memset(bitmap->map, -1, bitmap->size);
>>
>
> Other than few coding style issue, I think the 2 functions (or & and) 
> can be optimize (i.e working with byte rather than bit). Although, I'm 
> not a maintainers so I will let them decide if it's worth to do it.
>
> Regards,
>