Re: [PATCH] tools/libxl new bitmap functions

[Dario Faggioli <dario.faggioli@citrix.com>]

[-- Attachment #1.1: Type: text/plain, Size: 1767 bytes --]

On Thu, 2015-04-02 at 11:38 -0600, Linda Jacobson wrote:
> From: Linda <lindaj@jma3.com>
> 
> Added bitmap functions for union intersection and difference betweenn two bitmaps
> 
Just wondering, about union and intersection, are them the best names
for these operations, or do we want libxl_bitmap_or() and
libxl_bitmap_and()?  Personally, I'd go for _and() and _or(), it's more
common and more consistent with what we have already in the codebase.

About difference, what it the exact intended semantic of that operation?
In set theory, AFAICR, A-B is the set of elements that are in A and are
not in B. For a binary bitmap, I'd say it is the set of bits that are
set in A and are not set in B, is it so? Whatever it is, we should write
it down somewhere, e.g., in libxl_utils.h, as the function's doc
comment.

> --- a/tools/libxl/libxl_utils.c
> +++ b/tools/libxl/libxl_utils.c

> +int libxl_bitmap_intersection (libxl_ctx *ctx, libxl_bitmap 
> +*intersection_bitmap, libxl_bitmap *bitmap1, libxl_bitmap *bitmap2)
>
Lots of long lines. Limit is 80 charathers (and keeping them a little
less than that is even better :-))

To make that easier to happen, in this specific case (but also because
it is more convenient in general and requires less typing), I'd use
shorter variable names.

Perhaps not as much as ba and bb for the operands and bi for the result
(that would diverge from existing functions too much for my taste), but
at least bitmap1, bitmap2 and intersect, instead of intersection_bitmap.
Or even bitmap1 and bitmap2 for the operands, and just bitmap for the
result.

Of course, that does not guarantee that the fucntion signature stays on
one line. If it still does not, you should break it.

Regards,
Dario

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel