Re: [PATCH 2/3] util: Clear unused part of the map in virBitmapShrink

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

 



On Fri, Feb 02, 2018 at 08:23:33 +0100, Martin Kletzander wrote:
> Some of the other functions depend on the fact that unused bits and longs are
> always zero and it's less error-prone to clear it than fix the other functions.

Clearing the bitmap is okay with me, since if you grow it again it
should not magically re-gain it's old values.

Said that I think that also virBitmapNext*Bit functions should be fixed
anyways.

> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1540817
> 
> Signed-off-by: Martin Kletzander <mkletzan@xxxxxxxxxx>
> ---
>  src/util/virbitmap.c  | 13 +++++++++++++
>  tests/virbitmaptest.c |  5 +++++
>  2 files changed, 18 insertions(+)
> 
> diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c
> index b2c5c7a6a5ac..b32342024e19 100644
> --- a/src/util/virbitmap.c
> +++ b/src/util/virbitmap.c
> @@ -1213,9 +1213,22 @@ void
>  virBitmapShrink(virBitmapPtr map,
>                  size_t b)
>  {

I must say that I'm not a fan of the semantics of this API. The
expansion function makes sure that bit position 'b' is included in the
bitmap, while this makes sure that it's excluded.

> +    size_t nl = 0;
> +    size_t nb = 0;
> +
>      if (!map)
>          return;
>  
>      if (map->max_bit >= b)
>          map->max_bit = b;
> +
> +    nl = map->max_bit / VIR_BITMAP_BITS_PER_UNIT;
> +    nb = map->max_bit % VIR_BITMAP_BITS_PER_UNIT;
> +    map->map[nl] &= ((1UL << nb) - 1);
> +
> +    nl++;
> +    if (nl < map->map_len) {
> +        memset(map->map + nl, 0,
> +               (map->map_len - nl) * (VIR_BITMAP_BITS_PER_UNIT / CHAR_BIT));

Removing the memory allocation beyond the last bit should remove the
need to do this, since growing it back would then call the clearing
function and also remove potentially lingering memory.

If you don't have any strong opinion against shrinking the storage array
itself I'd prefer that solution.

Attachment: signature.asc
Description: PGP signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux