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>
>
>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
>>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list