From 154495a0c0d198e8e62e688eb9e1d9f9c9d7d897 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= <berrange@redhat.com>
Date: Thu, 22 Dec 2022 11:09:27 -0500
Subject: [PATCH] admin: use g_auto for client RPC return parameters
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Currently some, but not all, methods have a call to the
xdr_free function, for the 'ret' variable. This is done
on methods where there are complex structs containing
allocated memory. In other cases the structs contain
allocated memory, but the pointer is stolen, so xdr_free
is not called. In other cases no allocated memory is
present, so xdr_free.

This is hard to reason about, because the definition of
the struct is not visible in the client stubs.

Switch to use g_auto() for the 'ret' variable, which
means 'xdr_free' is always going to be called. Some
places now need to use g_steal_pointer as a result.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 src/admin/admin_remote.c | 50 +++++++++++-----------------------------
 1 file changed, 14 insertions(+), 36 deletions(-)

diff --git a/src/admin/admin_remote.c b/src/admin/admin_remote.c
index 3ae20ff373..3291a1e965 100644
--- a/src/admin/admin_remote.c
+++ b/src/admin/admin_remote.c
@@ -229,10 +229,9 @@ remoteAdminServerGetThreadPoolParameters(virAdmServerPtr srv,
                                          int *nparams,
                                          unsigned int flags)
 {
-    int rv = -1;
     remoteAdminPriv *priv = srv->conn->privateData;
     admin_server_get_threadpool_parameters_args args;
-    admin_server_get_threadpool_parameters_ret ret = {0};
+    g_auto(admin_server_get_threadpool_parameters_ret) ret = {0};
     VIR_LOCK_GUARD lock = virObjectLockGuard(priv);
 
     args.flags = flags;
@@ -248,13 +247,9 @@ remoteAdminServerGetThreadPoolParameters(virAdmServerPtr srv,
                                   ADMIN_SERVER_THREADPOOL_PARAMETERS_MAX,
                                   params,
                                   nparams) < 0)
-        goto cleanup;
+        return -1;
 
-    rv = 0;
-
- cleanup:
-    xdr_free((xdrproc_t)xdr_admin_server_get_threadpool_parameters_ret, (char *) &ret);
-    return rv;
+    return 0;
 }
 
 static int
@@ -297,10 +292,9 @@ remoteAdminClientGetInfo(virAdmClientPtr client,
                          int *nparams,
                          unsigned int flags)
 {
-    int rv = -1;
     remoteAdminPriv *priv = client->srv->conn->privateData;
     admin_client_get_info_args args;
-    admin_client_get_info_ret ret = {0};
+    g_auto(admin_client_get_info_ret) ret = {0};
     VIR_LOCK_GUARD lock = virObjectLockGuard(priv);
 
     args.flags = flags;
@@ -316,13 +310,9 @@ remoteAdminClientGetInfo(virAdmClientPtr client,
                                   ADMIN_CLIENT_INFO_PARAMETERS_MAX,
                                   params,
                                   nparams) < 0)
-        goto cleanup;
+        return -1;
 
-    rv = 0;
-
- cleanup:
-    xdr_free((xdrproc_t)xdr_admin_client_get_info_ret, (char *) &ret);
-    return rv;
+    return 0;
 }
 
 static int
@@ -331,9 +321,8 @@ remoteAdminServerGetClientLimits(virAdmServerPtr srv,
                                  int *nparams,
                                  unsigned int flags)
 {
-    int rv = -1;
     admin_server_get_client_limits_args args;
-    admin_server_get_client_limits_ret ret = {0};
+    g_auto(admin_server_get_client_limits_ret) ret = {0};
     remoteAdminPriv *priv = srv->conn->privateData;
     VIR_LOCK_GUARD lock = virObjectLockGuard(priv);
 
@@ -352,14 +341,9 @@ remoteAdminServerGetClientLimits(virAdmServerPtr srv,
                                   ADMIN_SERVER_CLIENT_LIMITS_MAX,
                                   params,
                                   nparams) < 0)
-        goto cleanup;
+        return -1;
 
-    rv = 0;
-
- cleanup:
-    xdr_free((xdrproc_t) xdr_admin_server_get_client_limits_ret,
-             (char *) &ret);
-    return rv;
+    return 0;
 }
 
 static int
@@ -401,10 +385,9 @@ remoteAdminConnectGetLoggingOutputs(virAdmConnectPtr conn,
                                     char **outputs,
                                     unsigned int flags)
 {
-    int rv = -1;
     remoteAdminPriv *priv = conn->privateData;
     admin_connect_get_logging_outputs_args args;
-    admin_connect_get_logging_outputs_ret ret = {0};
+    g_auto(admin_connect_get_logging_outputs_ret) ret = {0};
     VIR_LOCK_GUARD lock = virObjectLockGuard(priv);
 
     args.flags = flags;
@@ -421,9 +404,7 @@ remoteAdminConnectGetLoggingOutputs(virAdmConnectPtr conn,
     if (outputs)
         *outputs = g_steal_pointer(&ret.outputs);
 
-    rv = ret.noutputs;
-    xdr_free((xdrproc_t) xdr_admin_connect_get_logging_outputs_ret, (char *) &ret);
-    return rv;
+    return ret.noutputs;
 }
 
 static int
@@ -431,10 +412,9 @@ remoteAdminConnectGetLoggingFilters(virAdmConnectPtr conn,
                                     char **filters,
                                     unsigned int flags)
 {
-    int rv = -1;
     remoteAdminPriv *priv = conn->privateData;
     admin_connect_get_logging_filters_args args;
-    admin_connect_get_logging_filters_ret ret = {0};
+    g_auto(admin_connect_get_logging_filters_ret) ret = {0};
     VIR_LOCK_GUARD lock = virObjectLockGuard(priv);
 
     args.flags = flags;
@@ -449,9 +429,7 @@ remoteAdminConnectGetLoggingFilters(virAdmConnectPtr conn,
         return -1;
 
     if (filters)
-        *filters = ret.filters ? *ret.filters : NULL;
+        *filters = ret.filters ? g_steal_pointer(ret.filters) : NULL;
 
-    rv = ret.nfilters;
-    VIR_FREE(ret.filters);
-    return rv;
+    return ret.nfilters;
 }