[PATCH v4] admin: use g_autofree

Daniel Henrique Barboza danielhb413 at gmail.com
Wed Mar 4 18:32:00 UTC 2020



On 3/4/20 3:06 PM, Gaurav Agrawal wrote:
> Signed-off-by: Gaurav Agrawal <agrawalgaurav at gnome.org>
> ---
>   src/admin/libvirt-admin.c | 13 +++++--------
>   1 file changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/src/admin/libvirt-admin.c b/src/admin/libvirt-admin.c
> index 4099a54854..709e009467 100644
> --- a/src/admin/libvirt-admin.c
> +++ b/src/admin/libvirt-admin.c
> @@ -111,7 +111,7 @@ getSocketPath(virURIPtr uri)
>           virURIParamPtr param = &uri->params[i];
>   
>           if (STREQ(param->name, "socket")) {
> -            VIR_FREE(sock_path);
> +            g_free(sock_path);
>               sock_path = g_strdup(param->value);
>           } else {
>               virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> @@ -203,11 +203,11 @@ virAdmGetDefaultURI(virConfPtr conf, char **uristr)
>   virAdmConnectPtr
>   virAdmConnectOpen(const char *name, unsigned int flags)
>   {
> -    char *sock_path = NULL;
> +    g_autofree char *sock_path = NULL;
>       char *alias = NULL;
>       virAdmConnectPtr conn = NULL;
>       g_autoptr(virConf) conf = NULL;
> -    char *uristr = NULL;
> +    g_autofree char *uristr = NULL;
>   
>       if (virAdmInitialize() < 0)
>           goto error;
> @@ -233,7 +233,7 @@ virAdmConnectOpen(const char *name, unsigned int flags)
>           goto error;
>   
>       if (alias) {
> -        VIR_FREE(uristr);
> +        g_free(uristr);
>           uristr = alias;
>       }
>   
> @@ -251,16 +251,13 @@ virAdmConnectOpen(const char *name, unsigned int flags)
>       if (remoteAdminConnectOpen(conn, flags) < 0)
>           goto error;
>   
> - cleanup:
> -    VIR_FREE(sock_path);
> -    VIR_FREE(uristr);
>       return conn;
>   
>    error:
>       virDispatchError(NULL);
>       virObjectUnref(conn);
>       conn = NULL;


virObjectUnref(conn) will end up turning conn to NULL via 'VIR_FREE()', given
that 'conn' is guaranteed to have only one reference at this point (it was
just created via virAdmConnectNew() in this function).

This 'conn = NULL' assignment was probably being used here to make it clearer
that the 'goto cleanup' jump would return NULL in this situation. Since you're
removing the need for the jump and the label, this 'conn = NULL' assignment is
now unneeded.



The rest of the code LGTM. If the maintainer does not mind removing the 'conn = NULL'
when pushing the patch, here's my r-b:


Reviewed-by: Daniel Henrique Barboza <danielhb413 at gmail.com>




> -    goto cleanup;
> +    return NULL;
>   }
>   
>   /**
> 




More information about the libvir-list mailing list