Re: [PATCH] bitmap: Use g_try_new0/g_renew

[Daniel P . Berrangé]

On Thu, May 02, 2024 at 04:23:16PM +0200, Paolo Bonzini wrote:
> Avoids an explicit use of sizeof().  The GLib allocation macros
> ensure that the multiplication by the size of the element
> uses the right type and does not overflow.
> 
> Cc: qemu-trivial@nongnu.org
> Cc: Roman Kiryanov <rkir@google.com>
> Cc: Daniel Berrange <berrange@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  include/qemu/bitmap.h | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/include/qemu/bitmap.h b/include/qemu/bitmap.h
> index 97806811eeb..c4363b1d324 100644
> --- a/include/qemu/bitmap.h
> +++ b/include/qemu/bitmap.h
> @@ -92,8 +92,8 @@ long slow_bitmap_count_one(const unsigned long *bitmap, 
> long nbits);
>  
>  static inline unsigned long *bitmap_try_new(long nbits)
>  {
> -    long len = BITS_TO_LONGS(nbits) * sizeof(unsigned long);
> -    return g_try_malloc0(len);
> +    long nelem = BITS_TO_LONGS(nbits);
> +    return g_try_new0(unsigned long, nelem);
>  }
>  
>  static inline unsigned long *bitmap_new(long nbits)

I'd suggest this method be changed to directly call
g_new0 too. Its current impl of calling bitmap_try_new()
followed by a plain abort() has worse diagnostics than
g_new0, which uses g_error to report the actual allocation
size that failed.

> @@ -265,10 +265,10 @@ unsigned long bitmap_find_next_zero_area(unsigned long 
> *map,
>  static inline unsigned long *bitmap_zero_extend(unsigned long *old,
>                                                  long old_nbits, long 
> new_nbits)
>  {
> -    long new_len = BITS_TO_LONGS(new_nbits) * sizeof(unsigned long);
> -    unsigned long *new = g_realloc(old, new_len);
> -    bitmap_clear(new, old_nbits, new_nbits - old_nbits);
> -    return new;
> +    long new_nelem = BITS_TO_LONGS(new_nbits);
> +    unsigned long *ptr = g_renew(unsigned long, old, new_nelem);
> +    bitmap_clear(ptr, old_nbits, new_nbits - old_nbits);
> +    return ptr;
>  }
>  
>  void bitmap_to_le(unsigned long *dst, const unsigned long *src,
> -- 
> 2.44.0
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|