[libvirt] [RFC PATCH 1/1] src: remove NULL check before VIR_FREE()

[Daniel P. Berrangé]

On Tue, Jan 07, 2020 at 02:12:05PM -0300, Daniel Henrique Barboza wrote:
> Functions virDomainSoundCodecDefFree(), virCapabilitiesFreeStoragePool()
> and virStorageSourceNVMeDefFree() are doing 'if (ptr)' NULL
> checks right before a 'VIR_FREE(ptr)' call.
> 
> VIR_FREE() is a wrapper for g_free(). g_free(mem) documentation [1]
> states:
> 
> 'If mem is NULL it simply returns, so there is no need to check mem
> against NULL before calling this function.'
> 
> This patch removes these needless NULL checks. The
> virDomainSoundCodecDefFree() instance was found by accident
> via code inspection, the other 2 instances were found using
> Coccinelle.
> 
> [1] https://developer.gnome.org/glib/stable/glib-Memory-Allocation.html#g-free
> 
> Signed-off-by: Daniel Henrique Barboza <danielhb413 at gmail.com>
> ---
> 
> Sending this one as RFC because I am unsure about its utility.
> 
>  src/conf/capabilities.c   | 3 ---
>  src/conf/domain_conf.c    | 3 ---
>  src/util/virstoragefile.c | 3 ---
>  3 files changed, 9 deletions(-)
> 
> diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
> index 9a39858280..4414b61fb2 100644
> --- a/src/conf/capabilities.c
> +++ b/src/conf/capabilities.c
> @@ -174,9 +174,6 @@ virCapabilitiesFreeGuest(virCapsGuestPtr guest)
>  static void
>  virCapabilitiesFreeStoragePool(virCapsStoragePoolPtr pool)
>  {
> -    if (!pool)
> -        return;
> -
>      VIR_FREE(pool);
>  }

The idea behind this code pattern is that the structs grow
new fields over time. So someone might add

   VIR_FREE(pool->foo);

with the lines you are deleting, this function is now
crash-prone.

So we keep the check for NULL even though it might look
redundant now.

>  
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 6b05d06481..08e669daa9 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -2732,9 +2732,6 @@ void virDomainSmartcardDefFree(virDomainSmartcardDefPtr def)
>  
>  void virDomainSoundCodecDefFree(virDomainSoundCodecDefPtr def)
>  {
> -    if (!def)
> -        return;
> -
>      VIR_FREE(def);
>  }
>  
> diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
> index e46ac99219..e9e74ef124 100644
> --- a/src/util/virstoragefile.c
> +++ b/src/util/virstoragefile.c
> @@ -2121,9 +2121,6 @@ virStorageSourceNVMeDefIsEqual(const virStorageSourceNVMeDef *a,
>  void
>  virStorageSourceNVMeDefFree(virStorageSourceNVMeDefPtr def)
>  {
> -    if (!def)
> -        return;
> -
>      VIR_FREE(def);
>  }
>  
> -- 
> 2.24.1
> 
> 
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list

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