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

Re: [libvirt] [PATCH 09/21] util: Introduce virBitmapShrink

On Mon, Nov 13, 2017 at 03:42:40PM -0500, John Ferlan wrote:

On 11/13/2017 03:50 AM, Martin Kletzander wrote:
Sometimes the size of the bitmap matters and it might not be guessed correctly
when parsing from some type of input.  For example virBitmapNewData() has Byte
granularity, virBitmapNewString() has nibble granularity and so on.
virBitmapParseUnlimited() can be tricked into creating huge bitmap that's not
needed (e.g.: "0-2,^99999999").  This function provides a way to shrink the
bitmap.  It is not supposed to free any memory.

Is there a specific reason why you don't free memory?  Consider that the
corollary virBitmapExpand can always be used to regrow the bitmap. I'm
fine with not free'ing, but maybe someone would want to...  OK, sure
they can supply the patch some day, I know...

I don't free it because a) it would cost more time and b) we over-allocate a bit
anyway.  Also this is mainly used so that the bitmap size is predictable, not
much else.

Signed-off-by: Martin Kletzander <mkletzan redhat com>
 src/libvirt_private.syms |  1 +
 src/util/virbitmap.c     | 19 +++++++++++++++++++
 src/util/virbitmap.h     |  2 ++
 3 files changed, 22 insertions(+)

With a couple of notes below handled,

Reviewed-by: John Ferlan <jferlan redhat com>


diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 0b78a0681c5e..3986cc523e39 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1369,6 +1369,7 @@ virBitmapParseUnlimited;
diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c
index ac6ff4f6d26d..95b1f8656907 100644
--- a/src/util/virbitmap.c
+++ b/src/util/virbitmap.c
@@ -176,6 +176,25 @@ int virBitmapSetBit(virBitmapPtr bitmap, size_t b)
     return 0;

+ * virBitmapShrink:
+ * @map: Pointer to bitmap
+ * @b: last bit position to be excluded from bitmap
+ *
+ * Resizes the bitmap so that no more than @b bits will fit into it.  Nothing
+ * will change if the size is already smaller than @b.

Considering adding, "NB: Does not adjust the map->map_len so that a
subsequent virBitmapExpand doesn't necessarily need to reallocate."
(not required, just a suggestion)


+ */
+void virBitmapShrink(virBitmapPtr map, size_t b)

virBitmapStrink(virBitmapPtr map,
               size_t b)


+    if (!map)
+        return;
+    if (map->max_bit >= b)
+        map->max_bit = b;
  * virBitmapExpand:
  * @map: Pointer to bitmap
diff --git a/src/util/virbitmap.h b/src/util/virbitmap.h
index 7b2bea8b534c..2464814055de 100644
--- a/src/util/virbitmap.h
+++ b/src/util/virbitmap.h
@@ -153,4 +153,6 @@ void virBitmapIntersect(virBitmapPtr a, virBitmapPtr b)
 void virBitmapSubtract(virBitmapPtr a, virBitmapPtr b)

+void virBitmapShrink(virBitmapPtr map, size_t b);

Not that it matters but it's always nice to keep the .h file in the same
relative order as the .c file...  So this would move below virBitmapSetBit

I went the other way and moved it in the .c file, I have no idea why I didn't
put it in the end anyway.


Attachment: signature.asc
Description: Digital signature

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