[libvirt] [PATCH] storage: remove a redundant NULL assignment

Jiri Denemark jdenemar at redhat.com
Wed Nov 26 10:46:23 UTC 2014


On Wed, Nov 26, 2014 at 15:51:17 +0800, Chen Hanxiao wrote:
> We already did this in virSecretDefFree.
> 
> Signed-off-by: Chen Hanxiao <chenhanxiao at cn.fujitsu.com>
> ---
>  src/storage/storage_backend.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
> index 98720f6..440f8b1 100644
> --- a/src/storage/storage_backend.c
> +++ b/src/storage/storage_backend.c
> @@ -558,7 +558,6 @@ virStorageGenerateQcowEncryption(virConnectPtr conn,
>          goto cleanup;
>      xml = virSecretDefFormat(def);
>      virSecretDefFree(def);
> -    def = NULL;
>      if (xml == NULL)
>          goto cleanup;
>  

NACK, this patch could result in double-free. We do set def = NULL in
virSecretDefFree() but that only touches def internally visible to this
function. To make def from virStorageGenerateQcowEncryption be NULL
after virSecretDefFree, we'd have to pass def by reference to it, i.e.,
to call virSecretDefFree(&def) and change virSecretDefFree to count with
that. However, this is not a common practise so we shouldn't do it.
Unless we change all *Free functions to do it this way.

Jirka




More information about the libvir-list mailing list