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

[Martin Kletzander]

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

Added

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

done

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

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.

>>  #endif
>>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20171114/51fb78d1/attachment-0001.sig>