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

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



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


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