Clean up credential asking code in SSH transport

[PATCH 03/15] util: authconfig: Use automatic pointer clearing for virAuthConfig

Posted by Peter Krempa 3 years ago

Fix and clean up the error paths in virAuthConfigNew*.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 src/util/virauthconfig.c | 28 ++++++++--------------------
 1 file changed, 8 insertions(+), 20 deletions(-)

diff --git a/src/util/virauthconfig.c b/src/util/virauthconfig.c
index dabd7cd217..983ac47f6b 100644
--- a/src/util/virauthconfig.c
+++ b/src/util/virauthconfig.c
@@ -37,23 +37,17 @@ VIR_LOG_INIT("util.authconfig");

 virAuthConfig *virAuthConfigNew(const char *path)
 {
-    virAuthConfig *auth;
-
-    auth = g_new0(virAuthConfig, 1);
+    g_autoptr(virAuthConfig) auth = g_new0(virAuthConfig, 1);

     auth->path = g_strdup(path);

     if (!(auth->keyfile = g_key_file_new()))
-        goto error;
+        return NULL;

     if (!g_key_file_load_from_file(auth->keyfile, path, 0, NULL))
-        goto error;
-
-    return auth;
+        return NULL;

- error:
-    virAuthConfigFree(auth);
-    return NULL;
+    return g_steal_pointer(&auth);
 }


@@ -61,23 +55,17 @@ virAuthConfig *virAuthConfigNewData(const char *path,
                                       const char *data,
                                       size_t len)
 {
-    virAuthConfig *auth;
-
-    auth = g_new0(virAuthConfig, 1);
+    g_autoptr(virAuthConfig) auth = g_new0(virAuthConfig, 1);

     auth->path = g_strdup(path);

     if (!(auth->keyfile = g_key_file_new()))
-        goto error;
+        return NULL;

     if (!g_key_file_load_from_data(auth->keyfile, data, len, 0, NULL))
-        goto error;
-
-    return auth;
+        return NULL;

- error:
-    virAuthConfigFree(auth);
-    return NULL;
+    return g_steal_pointer(&auth);
 }


-- 
2.38.1

Re: [PATCH 03/15] util: authconfig: Use automatic pointer clearing for virAuthConfig

Posted by Jonathon Jongsma 3 years ago

On 1/17/23 10:20 AM, Peter Krempa wrote:
> Fix and clean up the error paths in virAuthConfigNew*.
> 
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
>   src/util/virauthconfig.c | 28 ++++++++--------------------
>   1 file changed, 8 insertions(+), 20 deletions(-)
> 
> diff --git a/src/util/virauthconfig.c b/src/util/virauthconfig.c
> index dabd7cd217..983ac47f6b 100644
> --- a/src/util/virauthconfig.c
> +++ b/src/util/virauthconfig.c
> @@ -37,23 +37,17 @@ VIR_LOG_INIT("util.authconfig");
> 
>   virAuthConfig *virAuthConfigNew(const char *path)
>   {
> -    virAuthConfig *auth;
> -
> -    auth = g_new0(virAuthConfig, 1);
> +    g_autoptr(virAuthConfig) auth = g_new0(virAuthConfig, 1);
> 
>       auth->path = g_strdup(path);
> 
>       if (!(auth->keyfile = g_key_file_new()))
> -        goto error;
> +        return NULL;

Unrelated to your changes, but as far as I know, g_key_file_new() cannot 
actually fail so this check is pointless.

> 
>       if (!g_key_file_load_from_file(auth->keyfile, path, 0, NULL))
> -        goto error;
> -
> -    return auth;
> +        return NULL;
> 
> - error:
> -    virAuthConfigFree(auth);
> -    return NULL;
> +    return g_steal_pointer(&auth);
>   }
> 
> 
> @@ -61,23 +55,17 @@ virAuthConfig *virAuthConfigNewData(const char *path,
>                                         const char *data,
>                                         size_t len)
>   {
> -    virAuthConfig *auth;
> -
> -    auth = g_new0(virAuthConfig, 1);
> +    g_autoptr(virAuthConfig) auth = g_new0(virAuthConfig, 1);
> 
>       auth->path = g_strdup(path);
> 
>       if (!(auth->keyfile = g_key_file_new()))
> -        goto error;
> +        return NULL;

again

> 
>       if (!g_key_file_load_from_data(auth->keyfile, data, len, 0, NULL))
> -        goto error;
> -
> -    return auth;
> +        return NULL;
> 
> - error:
> -    virAuthConfigFree(auth);
> -    return NULL;
> +    return g_steal_pointer(&auth);
>   }
> 
> 



Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>