From mboxrd@z Thu Jan 1 00:00:00 1970 From: Wei Liu Subject: Re: [PATCH] new functions libxl_bitmap_{or,and} Date: Tue, 14 Apr 2015 10:19:42 +0100 Message-ID: <20150414091942.GN17670@zion.uk.xensource.com> References: <1428911238-6244-1-git-send-email-lindaj@jma3.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <1428911238-6244-1-git-send-email-lindaj@jma3.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Linda Jacobson Cc: wei.liu2@citrix.com, ian.campbell@citrix.com, stefano.stabellini@eu.citrix.com, lars.kurth.xen@gmail.com, xen-devel@lists.xen.org, julien.grall@citrix.com, ian.jackson@citrix.com List-Id: xen-devel@lists.xenproject.org Thanks, we're getting there. If my comments confuse you please just ask for clarification. There is no need to change the subject line. However, it would be useful if you have some kind of version number in you subject line. That is [PATCH vX] libxl: provide libxl_bimap_{and,or} You can do this by supplying --subject-prefix= to git format-patch git format-patch -1 --subject-prefix="[PATCH vX]" ... where X refers to your version number. On Mon, Apr 13, 2015 at 01:47:18AM -0600, Linda Jacobson wrote: > provide logical and and or of two bitmaps > And the SoB line should be here. > --- > > v.1 updated comments and format > v.2 rewrote bitmap functions to manipulate bytes not bits > > Signed-off-by: Linda Jacobson > --- > tools/libxl/libxl_utils.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++ > tools/libxl/libxl_utils.h | 5 ++++ > 2 files changed, 79 insertions(+) > > diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c > index 9053b27..5c7178f 100644 > --- a/tools/libxl/libxl_utils.c > +++ b/tools/libxl/libxl_utils.c > @@ -691,6 +691,80 @@ void libxl_bitmap_reset(libxl_bitmap *bitmap, int bit) > bitmap->map[bit / 8] &= ~(1 << (bit & 7)); > } > > +/* provide logical or and logical and of two bitmaps */ Normally we take the line before as the comment for the function that follows it. So you don't need to mention "and" function here. Actually I don't think you need a comment for such obvious function at all. > +int libxl_bitmap_or(libxl_ctx *ctx, libxl_bitmap *or_map, > + libxl_bitmap *map1, libxl_bitmap *map2) Actually I think you need to constify map1 and map2. I.e. const libxl_bitmap *map1, const libxl_bitmap *map2) Sorry for not having mentioned these earlier. Since you're going to resend due to other issues so you might as well just take care of these nits. > +{ > + GC_INIT(ctx); > + int rc; > + uint32_t i; > + libxl_bitmap *lgmap; > + libxl_bitmap *smap; > + Constify these as well. And probably use better name like large_map and small_map? > + if (map1->size > map2->size) { > + lgmap = map1; > + smap = map2; > + } > + else { Coding style. Should be } else { > + lgmap = map2; > + smap = map1; > + } > + > + rc = libxl_bitmap_alloc(ctx, or_map, lgmap->size * 8); > + if (rc) > + goto out; > + > + /* > + * if bitmaps aren't the same size, their union (logical or) will If > + * be size of larger bit map. Any bit past the end of the > + * smaller bit map, will match the larger one. > + */ > + for (i = 0; i < smap->size; i++) > + or_map->map[i] = (smap->map[i] | lgmap->map[i]); > + > + for (i = smap->size; i < lgmap->size; i++) > + or_map->map[i] = lgmap->map[i]; > + > +out: > + GC_FREE; > + return rc; > +} > + > +int libxl_bitmap_and(libxl_ctx *ctx, libxl_bitmap *and_map, > + libxl_bitmap *map1, libxl_bitmap *map2) Constify map1 and map2. > +{ > + GC_INIT(ctx); > + int rc; > + uint32_t i; > + libxl_bitmap *lgmap, *smap; Constify lgmap and smap. > + > + if (map1->size > map2->size) { > + lgmap = map1; > + smap = map2; > + } > + else { Coding style. > + lgmap = map2; > + smap = map1; > + } > + > + > + rc = libxl_bitmap_alloc(ctx, and_map, smap->size * 8); > + if (rc) > + goto out; > + > + /* > + * if bitmaps aren't same size, their 'and' will be size of If > + * smaller bit map > + */ > + for (i = 0; i < and_map->size; i++) > + and_map->map[i] = (lgmap->map[i] & smap->map[i]); > + > +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 acacdd9..1c0086b 100644 > --- a/tools/libxl/libxl_utils.h > +++ b/tools/libxl/libxl_utils.h > @@ -90,6 +90,11 @@ int libxl_bitmap_test(const libxl_bitmap *bitmap, int bit); > 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 *cpumap); > +/* or and and functions for two bitmaps */ Or > +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); Constify map1 and map2. Wei. > char *libxl_bitmap_to_hex_string(libxl_ctx *ctx, const libxl_bitmap *cpumap); > static inline void libxl_bitmap_set_any(libxl_bitmap *bitmap) > { > -- > 1.9.1