[v1] Change return type of functions that use VIR_EXPAND_N and never fail to void

[PATCH] Change return type of functions that use VIR_EXPAND_N and never fail to void

Posted by Alexander Kuznetsov 1 week, 2 days ago

These functions return value is invariant since VIR_EXPAND_N check
removal in 7d2fd6e, so change its type and remove all dependent checks.

Found by Linux Verification Center (linuxtesting.org) with Svace.

Reported-by: Pavel Nekrasov <p.nekrasov@fobos-nt.ru>
Signed-off-by: Alexander Kuznetsov <kuznetsovam@altlinux.org>
---
 src/access/viraccessdriverstack.c |  4 +---
 src/access/viraccessdriverstack.h |  2 +-
 src/access/viraccessmanager.c     |  5 +----
 src/admin/admin_remote.c          |  3 +--
 src/hyperv/hyperv_wmi.c           | 13 ++++-------
 src/locking/lock_daemon.c         | 10 ++-------
 src/locking/lock_driver_lockd.c   |  3 +--
 src/logging/log_daemon.c          | 10 ++-------
 src/logging/log_manager.c         |  3 +--
 src/lxc/lxc_monitor.c             |  4 +---
 src/remote/remote_daemon.c        | 20 ++++-------------
 src/remote/remote_driver.c        | 17 +++++---------
 src/rpc/gendispatch.pl            |  5 +----
 src/rpc/virnetclient.c            |  6 ++---
 src/rpc/virnetclient.h            |  4 ++--
 src/rpc/virnetserver.c            |  3 +--
 src/rpc/virnetserver.h            |  2 +-
 src/util/virsysinfo.c             | 37 ++++++++++---------------------
 18 files changed, 43 insertions(+), 108 deletions(-)

diff --git a/src/access/viraccessdriverstack.c b/src/access/viraccessdriverstack.c
index fb9ea71665..9d6a0d4d1b 100644
--- a/src/access/viraccessdriverstack.c
+++ b/src/access/viraccessdriverstack.c
@@ -32,7 +32,7 @@ struct _virAccessDriverStackPrivate {
 };
 
 
-int virAccessDriverStackAppend(virAccessManager *manager,
+void virAccessDriverStackAppend(virAccessManager *manager,
                                virAccessManager *child)
 {
     virAccessDriverStackPrivate *priv = virAccessManagerGetPrivateData(manager);
@@ -40,8 +40,6 @@ int virAccessDriverStackAppend(virAccessManager *manager,
     VIR_EXPAND_N(priv->managers, priv->managersLen, 1);
 
     priv->managers[priv->managersLen-1] = child;
-
-    return 0;
 }
 
 
diff --git a/src/access/viraccessdriverstack.h b/src/access/viraccessdriverstack.h
index abcfc30ec3..f878ef1989 100644
--- a/src/access/viraccessdriverstack.h
+++ b/src/access/viraccessdriverstack.h
@@ -23,7 +23,7 @@
 #include "access/viraccessdriver.h"
 
 
-int virAccessDriverStackAppend(virAccessManager *manager,
+void virAccessDriverStackAppend(virAccessManager *manager,
                                virAccessManager *child);
 
 extern virAccessDriver accessDriverStack;
diff --git a/src/access/viraccessmanager.c b/src/access/viraccessmanager.c
index 481528c3b9..6d9fdee5f1 100644
--- a/src/access/viraccessmanager.c
+++ b/src/access/viraccessmanager.c
@@ -164,10 +164,7 @@ virAccessManager *virAccessManagerNewStack(const char **names)
         if (!child)
             goto error;
 
-        if (virAccessDriverStackAppend(manager, child) < 0) {
-            virObjectUnref(child);
-            goto error;
-        }
+        virAccessDriverStackAppend(manager, child);
     }
 
     return manager;
diff --git a/src/admin/admin_remote.c b/src/admin/admin_remote.c
index 3291a1e965..5c4913a76e 100644
--- a/src/admin/admin_remote.c
+++ b/src/admin/admin_remote.c
@@ -214,8 +214,7 @@ remoteAdminPrivNew(const char *sock_path)
                                                  NULL, 0, NULL)))
         goto error;
 
-    if (virNetClientAddProgram(priv->client, priv->program) < 0)
-        goto error;
+    virNetClientAddProgram(priv->client, priv->program);
 
     return priv;
  error:
diff --git a/src/hyperv/hyperv_wmi.c b/src/hyperv/hyperv_wmi.c
index 8bc376e22f..0b82f1f131 100644
--- a/src/hyperv/hyperv_wmi.c
+++ b/src/hyperv/hyperv_wmi.c
@@ -184,14 +184,12 @@ hypervFreeInvokeParams(hypervInvokeParamsList *params)
 }
 
 
-static inline int
+static inline void
 hypervCheckParams(hypervInvokeParamsList *params)
 {
     if (params->nbParams + 1 > params->nbAvailParams) {
         VIR_EXPAND_N(params->params, params->nbAvailParams, 5);
     }
-
-    return 0;
 }
 
 
@@ -212,8 +210,7 @@ hypervAddSimpleParam(hypervInvokeParamsList *params, const char *name,
 {
     hypervParam *p = NULL;
 
-    if (hypervCheckParams(params) < 0)
-        return -1;
+    hypervCheckParams(params);
 
     p = &params->params[params->nbParams];
     p->type = HYPERV_SIMPLE_PARAM;
@@ -245,8 +242,7 @@ hypervAddEprParam(hypervInvokeParamsList *params,
 {
     hypervParam *p = NULL;
 
-    if (hypervCheckParams(params) < 0)
-        return -1;
+    hypervCheckParams(params);
 
     p = &params->params[params->nbParams];
     p->type = HYPERV_EPR_PARAM;
@@ -333,8 +329,7 @@ hypervAddEmbeddedParam(hypervInvokeParamsList *params,
 {
     hypervParam *p = NULL;
 
-    if (hypervCheckParams(params) < 0)
-        return -1;
+    hypervCheckParams(params);
 
     p = &params->params[params->nbParams];
     p->type = HYPERV_EMBEDDED_PARAM;
diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c
index ba52ce7d77..0b86300b43 100644
--- a/src/locking/lock_daemon.c
+++ b/src/locking/lock_daemon.c
@@ -1066,10 +1066,7 @@ int main(int argc, char **argv) {
         goto cleanup;
     }
 
-    if (virNetServerAddProgram(lockSrv, lockProgram) < 0) {
-        ret = VIR_DAEMON_ERR_INIT;
-        goto cleanup;
-    }
+    virNetServerAddProgram(lockSrv, lockProgram);
 
     if (adminSrv != NULL) {
         if (!(adminProgram = virNetServerProgramNew(ADMIN_PROGRAM,
@@ -1079,10 +1076,7 @@ int main(int argc, char **argv) {
             ret = VIR_DAEMON_ERR_INIT;
             goto cleanup;
         }
-        if (virNetServerAddProgram(adminSrv, adminProgram) < 0) {
-            ret = VIR_DAEMON_ERR_INIT;
-            goto cleanup;
-        }
+        virNetServerAddProgram(adminSrv, adminProgram);
     }
 
     /* Disable error func, now logging is setup */
diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c
index d75302dd0a..0b6c720477 100644
--- a/src/locking/lock_driver_lockd.c
+++ b/src/locking/lock_driver_lockd.c
@@ -213,8 +213,7 @@ static virNetClient *virLockManagerLockDaemonConnectionNew(bool privileged,
                                          NULL)))
         goto error;
 
-    if (virNetClientAddProgram(client, *prog) < 0)
-        goto error;
+    virNetClientAddProgram(client, *prog);
 
     return client;
 
diff --git a/src/logging/log_daemon.c b/src/logging/log_daemon.c
index 5a9be4a44e..16a5dcea9a 100644
--- a/src/logging/log_daemon.c
+++ b/src/logging/log_daemon.c
@@ -864,10 +864,7 @@ int main(int argc, char **argv) {
         ret = VIR_DAEMON_ERR_INIT;
         goto cleanup;
     }
-    if (virNetServerAddProgram(logSrv, logProgram) < 0) {
-        ret = VIR_DAEMON_ERR_INIT;
-        goto cleanup;
-    }
+    virNetServerAddProgram(logSrv, logProgram);
 
     if (adminSrv != NULL) {
         if (!(adminProgram = virNetServerProgramNew(ADMIN_PROGRAM,
@@ -877,10 +874,7 @@ int main(int argc, char **argv) {
             ret = VIR_DAEMON_ERR_INIT;
             goto cleanup;
         }
-        if (virNetServerAddProgram(adminSrv, adminProgram) < 0) {
-            ret = VIR_DAEMON_ERR_INIT;
-            goto cleanup;
-        }
+        virNetServerAddProgram(adminSrv, adminProgram);
     }
 
     /* Disable error func, now logging is setup */
diff --git a/src/logging/log_manager.c b/src/logging/log_manager.c
index d8490f4e5a..19e23d65c5 100644
--- a/src/logging/log_manager.c
+++ b/src/logging/log_manager.c
@@ -88,8 +88,7 @@ virLogManagerConnect(bool privileged,
                                          NULL)))
         goto error;
 
-    if (virNetClientAddProgram(client, *prog) < 0)
-        goto error;
+    virNetClientAddProgram(client, *prog);
 
     VIR_FREE(daemonPath);
     VIR_FREE(logdpath);
diff --git a/src/lxc/lxc_monitor.c b/src/lxc/lxc_monitor.c
index 811d6685e5..cf2fd1897f 100644
--- a/src/lxc/lxc_monitor.c
+++ b/src/lxc/lxc_monitor.c
@@ -169,9 +169,7 @@ virLXCMonitor *virLXCMonitorNew(virDomainObj *vm,
                                                 mon)))
         goto error;
 
-    if (virNetClientAddProgram(mon->client,
-                               mon->program) < 0)
-        goto error;
+    virNetClientAddProgram(mon->client, mon->program);
 
     mon->vm = virObjectRef(vm);
     memcpy(&mon->cb, cb, sizeof(mon->cb));
diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c
index 9e82132654..1d079c7e4b 100644
--- a/src/remote/remote_daemon.c
+++ b/src/remote/remote_daemon.c
@@ -1063,10 +1063,7 @@ int main(int argc, char **argv) {
         ret = VIR_DAEMON_ERR_INIT;
         goto cleanup;
     }
-    if (virNetServerAddProgram(srv, remoteProgram) < 0) {
-        ret = VIR_DAEMON_ERR_INIT;
-        goto cleanup;
-    }
+    virNetServerAddProgram(srv, remoteProgram);
 
     if (!(lxcProgram = virNetServerProgramNew(LXC_PROGRAM,
                                               LXC_PROTOCOL_VERSION,
@@ -1075,10 +1072,7 @@ int main(int argc, char **argv) {
         ret = VIR_DAEMON_ERR_INIT;
         goto cleanup;
     }
-    if (virNetServerAddProgram(srv, lxcProgram) < 0) {
-        ret = VIR_DAEMON_ERR_INIT;
-        goto cleanup;
-    }
+    virNetServerAddProgram(srv, lxcProgram);
 
     if (!(qemuProgram = virNetServerProgramNew(QEMU_PROGRAM,
                                                QEMU_PROTOCOL_VERSION,
@@ -1087,10 +1081,7 @@ int main(int argc, char **argv) {
         ret = VIR_DAEMON_ERR_INIT;
         goto cleanup;
     }
-    if (virNetServerAddProgram(srv, qemuProgram) < 0) {
-        ret = VIR_DAEMON_ERR_INIT;
-        goto cleanup;
-    }
+    virNetServerAddProgram(srv, qemuProgram);
 
     if (!(srvAdm = virNetServerNew("admin", 1,
                                    config->admin_min_workers,
@@ -1120,10 +1111,7 @@ int main(int argc, char **argv) {
         ret = VIR_DAEMON_ERR_INIT;
         goto cleanup;
     }
-    if (virNetServerAddProgram(srvAdm, adminProgram) < 0) {
-        ret = VIR_DAEMON_ERR_INIT;
-        goto cleanup;
-    }
+    virNetServerAddProgram(srvAdm, adminProgram);
 
     if (timeout > 0) {
         if (virNetDaemonAutoShutdown(dmn, timeout) < 0)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index e76d9e9ba4..307f9ca945 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -1160,10 +1160,9 @@ doRemoteOpen(virConnectPtr conn,
                                                      conn)))
         goto error;
 
-    if (virNetClientAddProgram(priv->client, priv->remoteProgram) < 0 ||
-        virNetClientAddProgram(priv->client, priv->lxcProgram) < 0 ||
-        virNetClientAddProgram(priv->client, priv->qemuProgram) < 0)
-        goto error;
+    virNetClientAddProgram(priv->client, priv->remoteProgram);
+    virNetClientAddProgram(priv->client, priv->lxcProgram);
+    virNetClientAddProgram(priv->client, priv->qemuProgram);
 
     /* Try and authenticate with server */
     VIR_DEBUG("Trying authentication");
@@ -5664,10 +5663,7 @@ remoteDomainMigratePrepareTunnel3(virConnectPtr dconn,
                                         false)))
         return -1;
 
-    if (virNetClientAddStream(priv->client, netst) < 0) {
-        virObjectUnref(netst);
-        return -1;
-    }
+    virNetClientAddStream(priv->client, netst);
 
     st->driver = &remoteStreamDrv;
     st->privateData = netst;
@@ -6433,10 +6429,7 @@ remoteDomainMigratePrepareTunnel3Params(virConnectPtr dconn,
                                         false)))
         goto cleanup;
 
-    if (virNetClientAddStream(priv->client, netst) < 0) {
-        virObjectUnref(netst);
-        goto cleanup;
-    }
+    virNetClientAddStream(priv->client, netst);
 
     st->driver = &remoteStreamDrv;
     st->privateData = netst;
diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl
index c5842dc796..724a6aed6e 100755
--- a/src/rpc/gendispatch.pl
+++ b/src/rpc/gendispatch.pl
@@ -1822,10 +1822,7 @@ elsif ($mode eq "client") {
             print "    if (!(netst = virNetClientStreamNew(priv->remoteProgram, $call->{constname}, priv->counter, sparse)))\n";
             print "        goto cleanup;\n";
             print "\n";
-            print "    if (virNetClientAddStream(priv->client, netst) < 0) {\n";
-            print "        virObjectUnref(netst);\n";
-            print "        goto cleanup;\n";
-            print "    }";
+            print "    virNetClientAddStream(priv->client, netst);\n";
             print "\n";
             print "    st->driver = &remoteStreamDrv;\n";
             print "    st->privateData = netst;\n";
diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
index 6d424eb599..39ccbd739c 100644
--- a/src/rpc/virnetclient.c
+++ b/src/rpc/virnetclient.c
@@ -1053,7 +1053,7 @@ bool virNetClientIsOpen(virNetClient *client)
 }
 
 
-int virNetClientAddProgram(virNetClient *client,
+void virNetClientAddProgram(virNetClient *client,
                            virNetClientProgram *prog)
 {
     virObjectLock(client);
@@ -1062,11 +1062,10 @@ int virNetClientAddProgram(virNetClient *client,
     client->programs[client->nprograms-1] = virObjectRef(prog);
 
     virObjectUnlock(client);
-    return 0;
 }
 
 
-int virNetClientAddStream(virNetClient *client,
+void virNetClientAddStream(virNetClient *client,
                           virNetClientStream *st)
 {
     virObjectLock(client);
@@ -1075,7 +1074,6 @@ int virNetClientAddStream(virNetClient *client,
     client->streams[client->nstreams-1] = virObjectRef(st);
 
     virObjectUnlock(client);
-    return 0;
 }
 
 
diff --git a/src/rpc/virnetclient.h b/src/rpc/virnetclient.h
index 1647a6cc71..29099791a0 100644
--- a/src/rpc/virnetclient.h
+++ b/src/rpc/virnetclient.h
@@ -117,10 +117,10 @@ int virNetClientDupFD(virNetClient *client, bool cloexec);
 
 bool virNetClientHasPassFD(virNetClient *client);
 
-int virNetClientAddProgram(virNetClient *client,
+void virNetClientAddProgram(virNetClient *client,
                            virNetClientProgram *prog);
 
-int virNetClientAddStream(virNetClient *client,
+void virNetClientAddStream(virNetClient *client,
                           virNetClientStream *st);
 
 void virNetClientRemoveStream(virNetClient *client,
diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
index a6c6443c55..91219c3eed 100644
--- a/src/rpc/virnetserver.c
+++ b/src/rpc/virnetserver.c
@@ -768,7 +768,7 @@ virNetServerAddServiceUNIX(virNetServer *srv,
 }
 
 
-int
+void
 virNetServerAddProgram(virNetServer *srv,
                        virNetServerProgram *prog)
 {
@@ -776,7 +776,6 @@ virNetServerAddProgram(virNetServer *srv,
 
     VIR_EXPAND_N(srv->programs, srv->nprograms, 1);
     srv->programs[srv->nprograms-1] = virObjectRef(prog);
-    return 0;
 }
 
 
diff --git a/src/rpc/virnetserver.h b/src/rpc/virnetserver.h
index 7756a1dd6c..c14d8bd10f 100644
--- a/src/rpc/virnetserver.h
+++ b/src/rpc/virnetserver.h
@@ -84,7 +84,7 @@ int virNetServerAddServiceUNIX(virNetServer *srv,
                                size_t max_queued_clients,
                                size_t nrequests_client_max);
 
-int virNetServerAddProgram(virNetServer *srv,
+void virNetServerAddProgram(virNetServer *srv,
                            virNetServerProgram *prog);
 
 int virNetServerSetTLSContext(virNetServer *srv,
diff --git a/src/util/virsysinfo.c b/src/util/virsysinfo.c
index dbcbca62ed..3575669ac3 100644
--- a/src/util/virsysinfo.c
+++ b/src/util/virsysinfo.c
@@ -263,7 +263,7 @@ virSysinfoParsePPCSystem(const char *base, virSysinfoSystemDef **sysdef)
     return ret;
 }
 
-static int
+static void
 virSysinfoParsePPCProcessor(const char *base, virSysinfoDef *ret)
 {
     const char *cur;
@@ -303,8 +303,6 @@ virSysinfoParsePPCProcessor(const char *base, virSysinfoDef *ret)
         }
 
     }
-
-    return 0;
 }
 
 /* virSysinfoRead for PowerPC
@@ -325,8 +323,7 @@ virSysinfoReadPPC(void)
 
     ret->nprocessor = 0;
     ret->processor = NULL;
-    if (virSysinfoParsePPCProcessor(outbuf, ret) < 0)
-        return NULL;
+    virSysinfoParsePPCProcessor(outbuf, ret);
 
     if (virSysinfoParsePPCSystem(outbuf, &ret->system) < 0)
         return NULL;
@@ -383,7 +380,7 @@ virSysinfoParseARMSystem(const char *base, virSysinfoSystemDef **sysdef)
     return ret;
 }
 
-static int
+static void
 virSysinfoParseARMProcessor(const char *base, virSysinfoDef *ret)
 {
     const char *cur;
@@ -393,7 +390,7 @@ virSysinfoParseARMProcessor(const char *base, virSysinfoDef *ret)
 
     if (!(tmp_base = strstr(base, "model name")) &&
         !(tmp_base = strstr(base, "Processor")))
-        return 0;
+        return;
 
     eol = strchr(tmp_base, '\n');
     cur = strchr(tmp_base, ':') + 1;
@@ -420,7 +417,7 @@ virSysinfoParseARMProcessor(const char *base, virSysinfoDef *ret)
     }
 
     VIR_FREE(processor_type);
-    return 0;
+    return;
 }
 
 /* virSysinfoRead for ARMv7
@@ -451,8 +448,7 @@ virSysinfoReadARM(void)
 
     ret->nprocessor = 0;
     ret->processor = NULL;
-    if (virSysinfoParseARMProcessor(outbuf, ret) < 0)
-        return NULL;
+    virSysinfoParseARMProcessor(outbuf, ret);
 
     if (virSysinfoParseARMSystem(outbuf, &ret->system) < 0)
         return NULL;
@@ -753,7 +749,7 @@ virSysinfoParseX86System(const char *base, virSysinfoSystemDef **sysdef)
     return ret;
 }
 
-static int
+static void
 virSysinfoParseX86BaseBoard(const char *base,
                             virSysinfoBaseBoardDef **baseBoard,
                             size_t *nbaseBoard)
@@ -827,7 +823,6 @@ virSysinfoParseX86BaseBoard(const char *base,
 
     *nbaseBoard = nboards;
     *baseBoard = g_steal_pointer(&boards);
-    return 0;
 }
 
 
@@ -1006,7 +1001,7 @@ virSysinfoParseOEMStrings(const char *base,
 }
 
 
-static int
+static void
 virSysinfoParseX86Processor(const char *base, virSysinfoDef *ret)
 {
     const char *cur, *tmp_base;
@@ -1102,11 +1097,9 @@ virSysinfoParseX86Processor(const char *base, virSysinfoDef *ret)
 
         base += strlen("Processor Information");
     }
-
-    return 0;
 }
 
-static int
+static void
 virSysinfoParseX86Memory(const char *base, virSysinfoDef *ret)
 {
     const char *cur, *tmp_base;
@@ -1197,8 +1190,6 @@ virSysinfoParseX86Memory(const char *base, virSysinfoDef *ret)
     next:
         base += strlen("Memory Device");
     }
-
-    return 0;
 }
 
 virSysinfoDef *
@@ -1223,8 +1214,7 @@ virSysinfoReadDMI(void)
     if (virSysinfoParseX86System(outbuf, &ret->system) < 0)
         return NULL;
 
-    if (virSysinfoParseX86BaseBoard(outbuf, &ret->baseBoard, &ret->nbaseBoard) < 0)
-        return NULL;
+    virSysinfoParseX86BaseBoard(outbuf, &ret->baseBoard, &ret->nbaseBoard);
 
     if (virSysinfoParseX86Chassis(outbuf, &ret->chassis) < 0)
         return NULL;
@@ -1234,13 +1224,10 @@ virSysinfoReadDMI(void)
 
     ret->nprocessor = 0;
     ret->processor = NULL;
-    if (virSysinfoParseX86Processor(outbuf, ret) < 0)
-        return NULL;
-
+    virSysinfoParseX86Processor(outbuf, ret);
     ret->nmemory = 0;
     ret->memory = NULL;
-    if (virSysinfoParseX86Memory(outbuf, ret) < 0)
-        return NULL;
+    virSysinfoParseX86Memory(outbuf, ret);
 
     return g_steal_pointer(&ret);
 }
-- 
2.42.2

Re: [PATCH] Change return type of functions that use VIR_EXPAND_N and never fail to void

Posted by Peter Krempa 1 week, 1 day ago

On Wed, Nov 27, 2024 at 18:45:09 +0300, Alexander Kuznetsov wrote:
> These functions return value is invariant since VIR_EXPAND_N check
> removal in 7d2fd6e, so change its type and remove all dependent checks.
> 
> Found by Linux Verification Center (linuxtesting.org) with Svace.
> 
> Reported-by: Pavel Nekrasov <p.nekrasov@fobos-nt.ru>
> Signed-off-by: Alexander Kuznetsov <kuznetsovam@altlinux.org>
> ---

Same as in previous reply. Please do this either once per function or
once per file but doing all at once makes this commit too big.

[PATCH v2 0/6] Change return type of functions that use VIR_EXPAND_N and never fail to void