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

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

 



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@xxxxxxxxxxxxxx>
> ---
>  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

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