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

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




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

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

John

> 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;
>  virBitmapSetAll;
>  virBitmapSetBit;
>  virBitmapSetBitExpand;
> +virBitmapShrink;
>  virBitmapSize;
>  virBitmapSubtract;
>  virBitmapToData;
> 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)

void
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)
>      ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
>  
> +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

>  #endif
> 


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