[libvirt] [REPOST PATCH] util: Clean up consumers of virJSONValueArraySize

John Ferlan posted 1 patch 5 years, 10 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20180510154311.12882-1-jferlan@redhat.com
Test syntax-check passed
src/locking/lock_daemon.c     |  7 ++--
src/logging/log_handler.c     |  7 ++--
src/network/bridge_driver.c   |  7 ++--
src/qemu/qemu_agent.c         | 35 ++++++++++--------
src/qemu/qemu_monitor_json.c  | 85 ++++++++++++++++++++-----------------------
src/rpc/virnetdaemon.c        |  7 +---
src/rpc/virnetserver.c        | 15 +++-----
src/rpc/virnetserverservice.c |  7 ++--
src/util/virjson.c            |  5 +--
src/util/virjson.h            |  2 +-
src/util/virlockspace.c       | 16 ++++----
src/util/virstoragefile.c     |  3 +-
tests/testutilsqemuschema.c   | 18 +++------
tools/nss/libvirt_nss.c       |  3 +-
14 files changed, 97 insertions(+), 120 deletions(-)
[libvirt] [REPOST PATCH] util: Clean up consumers of virJSONValueArraySize
Posted by John Ferlan 5 years, 10 months ago
Rather than have virJSONValueArraySize return a -1 when the input
is not an array and then splat an error message, let's check for
an array before calling and then change the return to be a size_t
instead of ssize_t.

That means using the helper virJSONValueIsArray as well as using a
more generic error message such as "Malformed <something> array".
In some cases we can remove stack variables and when we cannot,
those variables should be size_t not ssize_t. Alter a few references
of if (!value) to be if (value == 0) instead as well.

Some callers can already assume an array is being worked on based
on the previous call, so there's less to do.

Signed-off-by: John Ferlan <jferlan@redhat.com>
---

 Original:
 https://www.redhat.com/archives/libvir-list/2018-April/msg02003.html

 Reposting as I found a recent upstream changes created some simple
 merge conflicts and it's never clear if applying what was there can
 be applied on top even with git am -3, so I figured I would just repost.

 As noted previously, based off a review of the Coverity work from Peter:

 https://www.redhat.com/archives/libvir-list/2018-April/msg01613.html

 src/locking/lock_daemon.c     |  7 ++--
 src/logging/log_handler.c     |  7 ++--
 src/network/bridge_driver.c   |  7 ++--
 src/qemu/qemu_agent.c         | 35 ++++++++++--------
 src/qemu/qemu_monitor_json.c  | 85 ++++++++++++++++++++-----------------------
 src/rpc/virnetdaemon.c        |  7 +---
 src/rpc/virnetserver.c        | 15 +++-----
 src/rpc/virnetserverservice.c |  7 ++--
 src/util/virjson.c            |  5 +--
 src/util/virjson.h            |  2 +-
 src/util/virlockspace.c       | 16 ++++----
 src/util/virstoragefile.c     |  3 +-
 tests/testutilsqemuschema.c   | 18 +++------
 tools/nss/libvirt_nss.c       |  3 +-
 14 files changed, 97 insertions(+), 120 deletions(-)

diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c
index 7afff42246..78c33bd29c 100644
--- a/src/locking/lock_daemon.c
+++ b/src/locking/lock_daemon.c
@@ -248,7 +248,6 @@ virLockDaemonNewPostExecRestart(virJSONValuePtr object, bool privileged)
     virJSONValuePtr child;
     virJSONValuePtr lockspaces;
     size_t i;
-    ssize_t n;
     const char *serverNames[] = { "virtlockd" };
 
     if (VIR_ALLOC(lockd) < 0)
@@ -281,13 +280,13 @@ virLockDaemonNewPostExecRestart(virJSONValuePtr object, bool privileged)
         goto error;
     }
 
-    if ((n = virJSONValueArraySize(lockspaces)) < 0) {
+    if (!virJSONValueIsArray(lockspaces)) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                       _("Malformed lockspaces data from JSON file"));
+                       _("Malformed lockspaces array"));
         goto error;
     }
 
-    for (i = 0; i < n; i++) {
+    for (i = 0; i < virJSONValueArraySize(lockspaces); i++) {
         virLockSpacePtr lockspace;
 
         child = virJSONValueArrayGet(lockspaces, i);
diff --git a/src/logging/log_handler.c b/src/logging/log_handler.c
index 40dfa8ecae..5364e06dff 100644
--- a/src/logging/log_handler.c
+++ b/src/logging/log_handler.c
@@ -292,7 +292,6 @@ virLogHandlerNewPostExecRestart(virJSONValuePtr object,
 {
     virLogHandlerPtr handler;
     virJSONValuePtr files;
-    ssize_t n;
     size_t i;
 
     if (!(handler = virLogHandlerNew(privileged,
@@ -308,13 +307,13 @@ virLogHandlerNewPostExecRestart(virJSONValuePtr object,
         goto error;
     }
 
-    if ((n = virJSONValueArraySize(files)) < 0) {
+    if (!virJSONValueIsArray(files)) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                       _("Malformed files data from JSON file"));
+                       _("Malformed files array"));
         goto error;
     }
 
-    for (i = 0; i < n; i++) {
+    for (i = 0; i < virJSONValueArraySize(files); i++) {
         virLogHandlerLogFilePtr file;
         virJSONValuePtr child = virJSONValueArrayGet(files, i);
 
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index ece7e41d35..4b33e12b30 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -4124,7 +4124,7 @@ networkGetDHCPLeases(virNetworkPtr net,
     size_t i, j;
     size_t nleases = 0;
     int rv = -1;
-    ssize_t size = 0;
+    size_t size = 0;
     int custom_lease_file_len = 0;
     bool need_results = !!leases;
     long long currtime = 0;
@@ -4179,11 +4179,12 @@ networkGetDHCPLeases(virNetworkPtr net,
             goto error;
         }
 
-        if ((size = virJSONValueArraySize(leases_array)) < 0) {
+        if (!virJSONValueIsArray(leases_array)) {
             virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                           _("couldn't fetch array of leases"));
+                           _("Malformed lease_entries array"));
             goto error;
         }
+        size = virJSONValueArraySize(leases_array);
     }
 
     currtime = (long long)time(NULL);
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index 4df1bde458..eeede645ef 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -1490,7 +1490,7 @@ qemuAgentGetVCPUs(qemuAgentPtr mon,
     virJSONValuePtr cmd;
     virJSONValuePtr reply = NULL;
     virJSONValuePtr data = NULL;
-    ssize_t ndata;
+    size_t ndata;
 
     if (!(cmd = qemuAgentMakeCommand("guest-get-vcpus", NULL)))
         return -1;
@@ -1505,6 +1505,12 @@ qemuAgentGetVCPUs(qemuAgentPtr mon,
         goto cleanup;
     }
 
+    if (!virJSONValueIsArray(data)) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("Malformed guest-get-vcpus data array"));
+        goto cleanup;
+    }
+
     ndata = virJSONValueArraySize(data);
 
     if (VIR_ALLOC_N(*info, ndata) < 0)
@@ -1869,15 +1875,14 @@ qemuAgentGetFSInfo(qemuAgentPtr mon, virDomainFSInfoPtr **info,
         goto cleanup;
     }
 
-    if (virJSONValueGetType(data) != VIR_JSON_TYPE_ARRAY) {
+    if (!virJSONValueIsArray(data)) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                       _("guest-get-fsinfo return information was not "
-                         "an array"));
+                       _("Malformed guest-get-fsinfo data array"));
         goto cleanup;
     }
 
     ndata = virJSONValueArraySize(data);
-    if (!ndata) {
+    if (ndata == 0) {
         ret = 0;
         *info = NULL;
         goto cleanup;
@@ -1928,14 +1933,14 @@ qemuAgentGetFSInfo(qemuAgentPtr mon, virDomainFSInfoPtr **info,
             goto cleanup;
         }
 
-        if (virJSONValueGetType(entry) != VIR_JSON_TYPE_ARRAY) {
+        if (!virJSONValueIsArray(entry)) {
             virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                           _("guest-get-fsinfo 'disk' data was not an array"));
+                           _("Malformed guest-get-fsinfo 'disk' data array"));
             goto cleanup;
         }
 
         ndisk = virJSONValueArraySize(entry);
-        if (!ndisk)
+        if (ndisk == 0)
             continue;
         if (VIR_ALLOC_N(info_ret[i]->devAlias, ndisk) < 0)
             goto cleanup;
@@ -2035,7 +2040,6 @@ qemuAgentGetInterfaces(qemuAgentPtr mon,
 {
     int ret = -1;
     size_t i, j;
-    ssize_t size = -1;
     virJSONValuePtr cmd = NULL;
     virJSONValuePtr reply = NULL;
     virJSONValuePtr ret_array = NULL;
@@ -2065,17 +2069,16 @@ qemuAgentGetInterfaces(qemuAgentPtr mon,
         goto cleanup;
     }
 
-    if ((size = virJSONValueArraySize(ret_array)) < 0) {
+    if (!virJSONValueIsArray(ret_array)) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                        _("qemu agent didn't return an array of interfaces"));
         goto cleanup;
     }
 
-    for (i = 0; i < size; i++) {
+    for (i = 0; i < virJSONValueArraySize(ret_array); i++) {
         virJSONValuePtr tmp_iface = virJSONValueArrayGet(ret_array, i);
         virJSONValuePtr ip_addr_arr = NULL;
         const char *hwaddr, *ifname_s, *name = NULL;
-        ssize_t ip_addr_arr_size;
         virDomainInterfacePtr iface = NULL;
 
         /* Shouldn't happen but doesn't hurt to check neither */
@@ -2131,14 +2134,16 @@ qemuAgentGetInterfaces(qemuAgentPtr mon,
         if (!ip_addr_arr)
             continue;
 
-        if ((ip_addr_arr_size = virJSONValueArraySize(ip_addr_arr)) < 0)
-            /* Mmm, empty 'ip-address'? */
+        if (!virJSONValueIsArray(ip_addr_arr)) {
+            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                           _("Malformed ip-addresses array"));
             goto error;
+        }
 
         /* If current iface already exists, continue with the count */
         addrs_count = iface->naddrs;
 
-        for (j = 0; j < ip_addr_arr_size; j++) {
+        for (j = 0; j < virJSONValueArraySize(ip_addr_arr); j++) {
             const char *type, *addr;
             virJSONValuePtr ip_addr_obj = virJSONValueArrayGet(ip_addr_arr, j);
             virDomainIPAddressPtr ip_addr;
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 8176175894..e0ea553c41 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -1627,9 +1627,9 @@ qemuMonitorJSONExtractCPUInfo(virJSONValuePtr data,
     struct qemuMonitorQueryCpusEntry *cpus = NULL;
     int ret = -1;
     size_t i;
-    ssize_t ncpus;
+    size_t ncpus;
 
-    if ((ncpus = virJSONValueArraySize(data)) <= 0)
+    if ((ncpus = virJSONValueArraySize(data)) == 0)
         return -2;
 
     if (VIR_ALLOC_N(cpus, ncpus) < 0)
@@ -3595,7 +3595,7 @@ qemuMonitorJSONQueryRxFilterParse(virJSONValuePtr msg,
     int ret = -1;
     const char *tmp;
     virJSONValuePtr returnArray, entry, table, element;
-    ssize_t nTable;
+    size_t nTable;
     size_t i;
     virNetDevRxFilterPtr fil = virNetDevRxFilterNew();
 
@@ -3656,12 +3656,13 @@ qemuMonitorJSONQueryRxFilterParse(virJSONValuePtr msg,
         goto cleanup;
     }
     if ((!(table = virJSONValueObjectGet(entry, "unicast-table"))) ||
-        ((nTable = virJSONValueArraySize(table)) < 0)) {
+        (!virJSONValueIsArray(table))) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                        _("Missing or invalid 'unicast-table' array "
                          "in query-rx-filter response"));
         goto cleanup;
     }
+    nTable = virJSONValueArraySize(table);
     if (VIR_ALLOC_N(fil->unicast.table, nTable))
         goto cleanup;
     for (i = 0; i < nTable; i++) {
@@ -3697,12 +3698,13 @@ qemuMonitorJSONQueryRxFilterParse(virJSONValuePtr msg,
         goto cleanup;
     }
     if ((!(table = virJSONValueObjectGet(entry, "multicast-table"))) ||
-        ((nTable = virJSONValueArraySize(table)) < 0)) {
+        (!virJSONValueIsArray(table))) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                        _("Missing or invalid 'multicast-table' array "
                          "in query-rx-filter response"));
         goto cleanup;
     }
+    nTable = virJSONValueArraySize(table);
     if (VIR_ALLOC_N(fil->multicast.table, nTable))
         goto cleanup;
     for (i = 0; i < nTable; i++) {
@@ -3731,12 +3733,13 @@ qemuMonitorJSONQueryRxFilterParse(virJSONValuePtr msg,
         goto cleanup;
     }
     if ((!(table = virJSONValueObjectGet(entry, "vlan-table"))) ||
-        ((nTable = virJSONValueArraySize(table)) < 0)) {
+        (!virJSONValueIsArray(table))) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                        _("Missing or invalid 'vlan-table' array "
                          "in query-rx-filter response"));
         goto cleanup;
     }
+    nTable = virJSONValueArraySize(table);
     if (VIR_ALLOC_N(fil->vlan.table, nTable))
         goto cleanup;
     for (i = 0; i < nTable; i++) {
@@ -4575,7 +4578,7 @@ qemuMonitorJSONGetAllBlockJobInfo(qemuMonitorPtr mon)
     virJSONValuePtr cmd = NULL;
     virJSONValuePtr reply = NULL;
     virJSONValuePtr data;
-    ssize_t nr_results;
+    size_t nr_results;
     size_t i;
     virHashTablePtr blockJobs = NULL;
 
@@ -4591,12 +4594,7 @@ qemuMonitorJSONGetAllBlockJobInfo(qemuMonitorPtr mon)
         goto cleanup;
     }
 
-    if ((nr_results = virJSONValueArraySize(data)) < 0) {
-        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                       _("unable to determine array size"));
-        goto cleanup;
-    }
-
+    nr_results = virJSONValueArraySize(data);
     if (!(blockJobs = virHashCreate(nr_results, virHashValueFree)))
         goto cleanup;
 
@@ -5114,7 +5112,7 @@ int qemuMonitorJSONGetMachines(qemuMonitorPtr mon,
     virJSONValuePtr reply = NULL;
     virJSONValuePtr data;
     qemuMonitorMachineInfoPtr *infolist = NULL;
-    ssize_t n = 0;
+    size_t n = 0;
     size_t i;
 
     *machines = NULL;
@@ -5206,7 +5204,7 @@ qemuMonitorJSONGetCPUDefinitions(qemuMonitorPtr mon,
     virJSONValuePtr reply = NULL;
     virJSONValuePtr data;
     qemuMonitorCPUDefInfoPtr *cpulist = NULL;
-    int n = 0;
+    size_t n = 0;
     size_t i;
 
     *cpus = NULL;
@@ -5257,7 +5255,7 @@ qemuMonitorJSONGetCPUDefinitions(qemuMonitorPtr mon,
         if (virJSONValueObjectHasKey(child, "unavailable-features")) {
             virJSONValuePtr blockers;
             size_t j;
-            int len;
+            size_t len;
 
             blockers = virJSONValueObjectGetArray(child,
                                                   "unavailable-features");
@@ -5494,7 +5492,7 @@ int qemuMonitorJSONGetCommands(qemuMonitorPtr mon,
     virJSONValuePtr reply = NULL;
     virJSONValuePtr data;
     char **commandlist = NULL;
-    ssize_t n = 0;
+    size_t n = 0;
     size_t i;
 
     *commands = NULL;
@@ -5550,7 +5548,7 @@ int qemuMonitorJSONGetEvents(qemuMonitorPtr mon,
     virJSONValuePtr reply = NULL;
     virJSONValuePtr data;
     char **eventlist = NULL;
-    ssize_t n = 0;
+    size_t n = 0;
     size_t i;
 
     *events = NULL;
@@ -5614,7 +5612,7 @@ qemuMonitorJSONGetCommandLineOptionParameters(qemuMonitorPtr mon,
     virJSONValuePtr data = NULL;
     virJSONValuePtr array = NULL;
     char **paramlist = NULL;
-    ssize_t n = 0;
+    size_t n = 0;
     size_t i;
 
     *params = NULL;
@@ -5646,17 +5644,17 @@ qemuMonitorJSONGetCommandLineOptionParameters(qemuMonitorPtr mon,
                              "return data"));
             goto cleanup;
         }
-        qemuMonitorSetOptions(mon, array);
-    }
 
-    if ((n = virJSONValueArraySize(array)) < 0) {
-        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                       _("query-command-line-options reply data was not "
-                         "an array"));
-        goto cleanup;
+        if (!virJSONValueIsArray(array)) {
+            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                           _("Malformed query-cmmand-line-options array"));
+            goto cleanup;
+        }
+
+        qemuMonitorSetOptions(mon, array);
     }
 
-    for (i = 0; i < n; i++) {
+    for (i = 0; i < virJSONValueArraySize(array); i++) {
         virJSONValuePtr child = virJSONValueArrayGet(array, i);
         const char *tmp;
 
@@ -5681,12 +5679,12 @@ qemuMonitorJSONGetCommandLineOptionParameters(qemuMonitorPtr mon,
     if (found)
         *found = true;
 
-    if ((n = virJSONValueArraySize(data)) < 0) {
+    if (!virJSONValueIsArray(data)) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                       _("query-command-line-options parameter data was not "
-                         "an array"));
+                       _("Malformed query-cmmand-line-options parameters array"));
         goto cleanup;
     }
+    n = virJSONValueArraySize(data);
 
     /* null-terminated list */
     if (VIR_ALLOC_N(paramlist, n + 1) < 0)
@@ -5776,7 +5774,7 @@ int qemuMonitorJSONGetObjectTypes(qemuMonitorPtr mon,
     virJSONValuePtr reply = NULL;
     virJSONValuePtr data;
     char **typelist = NULL;
-    ssize_t n = 0;
+    size_t n = 0;
     size_t i;
 
     *types = NULL;
@@ -5832,7 +5830,7 @@ int qemuMonitorJSONGetObjectListPaths(qemuMonitorPtr mon,
     virJSONValuePtr reply = NULL;
     virJSONValuePtr data;
     qemuMonitorJSONListPathPtr *pathlist = NULL;
-    ssize_t n = 0;
+    size_t n = 0;
     size_t i;
 
     *paths = NULL;
@@ -6062,7 +6060,7 @@ int qemuMonitorJSONGetDeviceProps(qemuMonitorPtr mon,
     virJSONValuePtr reply = NULL;
     virJSONValuePtr data;
     char **proplist = NULL;
-    ssize_t n = 0;
+    size_t n = 0;
     size_t i;
 
     *props = NULL;
@@ -6161,7 +6159,7 @@ qemuMonitorJSONGetMigrationCapabilities(qemuMonitorPtr mon,
     virJSONValuePtr caps;
     char **list = NULL;
     size_t i;
-    ssize_t n;
+    size_t n;
 
     *capabilities = NULL;
 
@@ -6272,7 +6270,7 @@ qemuMonitorJSONGetGICCapabilities(qemuMonitorPtr mon,
     virJSONValuePtr caps;
     virGICCapability *list = NULL;
     size_t i;
-    ssize_t n;
+    size_t n;
 
     *capabilities = NULL;
 
@@ -6495,7 +6493,7 @@ qemuMonitorJSONGetStringArray(qemuMonitorPtr mon, const char *qmpCmd,
     virJSONValuePtr reply = NULL;
     virJSONValuePtr data;
     char **list = NULL;
-    ssize_t n = 0;
+    size_t n = 0;
     size_t i;
 
     *array = NULL;
@@ -6892,14 +6890,11 @@ qemuMonitorJSONParseCPUx86Features(virJSONValuePtr data)
     virCPUDataPtr cpudata = NULL;
     virCPUx86CPUID cpuid;
     size_t i;
-    ssize_t n;
-
-    n = virJSONValueArraySize(data);
 
     if (!(cpudata = virCPUDataNew(VIR_ARCH_X86_64)))
         goto error;
 
-    for (i = 0; i < n; i++) {
+    for (i = 0; i < virJSONValueArraySize(data); i++) {
         if (qemuMonitorJSONParseCPUx86FeatureWord(virJSONValueArrayGet(data, i),
                                                   &cpuid) < 0 ||
             virCPUx86DataAddCPUID(cpudata, &cpuid) < 0)
@@ -6960,7 +6955,7 @@ qemuMonitorJSONCheckCPUx86(qemuMonitorPtr mon)
     virJSONValuePtr reply = NULL;
     virJSONValuePtr data;
     size_t i;
-    ssize_t n;
+    size_t n;
     int ret = -1;
 
     if (!(cmd = qemuMonitorJSONMakeCommand("qom-list",
@@ -7098,7 +7093,7 @@ qemuMonitorJSONGetIOThreads(qemuMonitorPtr mon,
     virJSONValuePtr reply = NULL;
     virJSONValuePtr data;
     qemuMonitorIOThreadInfoPtr *infolist = NULL;
-    ssize_t n = 0;
+    size_t n = 0;
     size_t i;
 
     *iothreads = NULL;
@@ -7180,7 +7175,6 @@ qemuMonitorJSONGetMemoryDeviceInfo(qemuMonitorPtr mon,
     virJSONValuePtr reply = NULL;
     virJSONValuePtr data = NULL;
     qemuMonitorMemoryDeviceInfoPtr meminfo = NULL;
-    ssize_t n;
     size_t i;
 
     if (!(cmd = qemuMonitorJSONMakeCommand("query-memory-devices", NULL)))
@@ -7198,9 +7192,8 @@ qemuMonitorJSONGetMemoryDeviceInfo(qemuMonitorPtr mon,
         goto cleanup;
 
     data = virJSONValueObjectGetArray(reply, "return");
-    n = virJSONValueArraySize(data);
 
-    for (i = 0; i < n; i++) {
+    for (i = 0; i < virJSONValueArraySize(data); i++) {
         virJSONValuePtr elem = virJSONValueArrayGet(data, i);
         const char *type;
 
@@ -7666,7 +7659,7 @@ qemuMonitorJSONGetHotpluggableCPUs(qemuMonitorPtr mon,
                                    size_t *nentries)
 {
     struct qemuMonitorQueryHotpluggableCpusEntry *info = NULL;
-    ssize_t ninfo = 0;
+    size_t ninfo = 0;
     int ret = -1;
     size_t i;
     virJSONValuePtr data;
diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c
index eb0b822545..31b687de12 100644
--- a/src/rpc/virnetdaemon.c
+++ b/src/rpc/virnetdaemon.c
@@ -322,12 +322,7 @@ virNetDaemonNewPostExecRestart(virJSONValuePtr object,
             goto error;
     } else if (virJSONValueIsArray(servers)) {
         size_t i;
-        ssize_t n = virJSONValueArraySize(servers);
-        if (n < 0) {
-            virReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("Server count %zd should be positive"), n);
-            goto error;
-        }
+        size_t n = virJSONValueArraySize(servers);
         if (n > nDefServerNames) {
             virReportError(VIR_ERR_INTERNAL_ERROR,
                            _("Server count %zd greater than default name count %zu"),
diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
index f4105b1394..5aeb188900 100644
--- a/src/rpc/virnetserver.c
+++ b/src/rpc/virnetserver.c
@@ -411,7 +411,6 @@ virNetServerPtr virNetServerNewPostExecRestart(virJSONValuePtr object,
     virJSONValuePtr clients;
     virJSONValuePtr services;
     size_t i;
-    ssize_t n;
     unsigned int min_workers;
     unsigned int max_workers;
     unsigned int priority_workers;
@@ -492,14 +491,13 @@ virNetServerPtr virNetServerNewPostExecRestart(virJSONValuePtr object,
         goto error;
     }
 
-    n =  virJSONValueArraySize(services);
-    if (n < 0) {
+    if (!virJSONValueIsArray(services)) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                       _("Malformed services data in JSON document"));
+                       _("Malformed services array"));
         goto error;
     }
 
-    for (i = 0; i < n; i++) {
+    for (i = 0; i < virJSONValueArraySize(services); i++) {
         virNetServerServicePtr service;
         virJSONValuePtr child = virJSONValueArrayGet(services, i);
         if (!child) {
@@ -525,14 +523,13 @@ virNetServerPtr virNetServerNewPostExecRestart(virJSONValuePtr object,
         goto error;
     }
 
-    n =  virJSONValueArraySize(clients);
-    if (n < 0) {
+    if (!virJSONValueIsArray(clients)) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                       _("Malformed clients data in JSON document"));
+                       _("Malformed clients array"));
         goto error;
     }
 
-    for (i = 0; i < n; i++) {
+    for (i = 0; i < virJSONValueArraySize(clients); i++) {
         virNetServerClientPtr client;
         virJSONValuePtr child = virJSONValueArrayGet(clients, i);
         if (!child) {
diff --git a/src/rpc/virnetserverservice.c b/src/rpc/virnetserverservice.c
index fb19d09dda..23fc23cab4 100644
--- a/src/rpc/virnetserverservice.c
+++ b/src/rpc/virnetserverservice.c
@@ -325,7 +325,7 @@ virNetServerServicePtr virNetServerServiceNewPostExecRestart(virJSONValuePtr obj
     virNetServerServicePtr svc;
     virJSONValuePtr socks;
     size_t i;
-    ssize_t n;
+    size_t n;
     unsigned int max;
 
     if (virNetServerServiceInitialize() < 0)
@@ -358,12 +358,13 @@ virNetServerServicePtr virNetServerServiceNewPostExecRestart(virJSONValuePtr obj
         goto error;
     }
 
-    if ((n = virJSONValueArraySize(socks)) < 0) {
+    if (!virJSONValueIsArray(socks)) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                       _("socks field in JSON was not an array"));
+                       _("Malformed socks array"));
         goto error;
     }
 
+    n = virJSONValueArraySize(socks);
     if (VIR_ALLOC_N(svc->socks, n) < 0)
         goto error;
     svc->nsocks = n;
diff --git a/src/util/virjson.c b/src/util/virjson.c
index dfe00d9280..bb4e3c257b 100644
--- a/src/util/virjson.c
+++ b/src/util/virjson.c
@@ -971,12 +971,9 @@ virJSONValueIsArray(virJSONValuePtr array)
 }
 
 
-ssize_t
+size_t
 virJSONValueArraySize(const virJSONValue *array)
 {
-    if (array->type != VIR_JSON_TYPE_ARRAY)
-        return -1;
-
     return array->data.array.nvalues;
 }
 
diff --git a/src/util/virjson.h b/src/util/virjson.h
index 0f098892b4..e4a82bdbc8 100644
--- a/src/util/virjson.h
+++ b/src/util/virjson.h
@@ -81,7 +81,7 @@ virJSONValuePtr virJSONValueObjectGetByType(virJSONValuePtr object,
 bool virJSONValueIsObject(virJSONValuePtr object);
 
 bool virJSONValueIsArray(virJSONValuePtr array);
-ssize_t virJSONValueArraySize(const virJSONValue *array);
+size_t virJSONValueArraySize(const virJSONValue *array);
 virJSONValuePtr virJSONValueArrayGet(virJSONValuePtr object, unsigned int element);
 virJSONValuePtr virJSONValueArraySteal(virJSONValuePtr object, unsigned int element);
 typedef int (*virJSONArrayIteratorFunc)(size_t pos,
diff --git a/src/util/virlockspace.c b/src/util/virlockspace.c
index 41af0cdb6d..3364c843aa 100644
--- a/src/util/virlockspace.c
+++ b/src/util/virlockspace.c
@@ -293,7 +293,6 @@ virLockSpacePtr virLockSpaceNewPostExecRestart(virJSONValuePtr object)
 {
     virLockSpacePtr lockspace;
     virJSONValuePtr resources;
-    ssize_t n;
     size_t i;
 
     VIR_DEBUG("object=%p", object);
@@ -324,19 +323,19 @@ virLockSpacePtr virLockSpaceNewPostExecRestart(virJSONValuePtr object)
         goto error;
     }
 
-    if ((n = virJSONValueArraySize(resources)) < 0) {
+    if (!virJSONValueIsArray(resources)) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                       _("Malformed resources value in JSON document"));
+                       _("Malformed resources array"));
         goto error;
     }
 
-    for (i = 0; i < n; i++) {
+    for (i = 0; i < virJSONValueArraySize(resources); i++) {
         virJSONValuePtr child = virJSONValueArrayGet(resources, i);
         virLockSpaceResourcePtr res;
         const char *tmp;
         virJSONValuePtr owners;
         size_t j;
-        ssize_t m;
+        size_t m;
 
         if (VIR_ALLOC(res) < 0)
             goto error;
@@ -396,18 +395,19 @@ virLockSpacePtr virLockSpaceNewPostExecRestart(virJSONValuePtr object)
             goto error;
         }
 
-        if ((m = virJSONValueArraySize(owners)) < 0) {
+        if (!virJSONValueIsArray(owners)) {
             virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                           _("Malformed owners value in JSON document"));
+                           _("Malformed owners array"));
             virLockSpaceResourceFree(res);
             goto error;
         }
 
-        res->nOwners = m;
+        m = virJSONValueArraySize(owners);
         if (VIR_ALLOC_N(res->owners, res->nOwners) < 0) {
             virLockSpaceResourceFree(res);
             goto error;
         }
+        res->nOwners = m;
 
         for (j = 0; j < res->nOwners; j++) {
             unsigned long long int owner;
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index eae1dc01dc..275ab9b54b 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -2928,8 +2928,7 @@ virStorageSourceParseBackingJSONGluster(virStorageSourcePtr src,
         return -1;
 
     nservers = virJSONValueArraySize(server);
-
-    if (nservers < 1) {
+    if (nservers == 0) {
         virReportError(VIR_ERR_INVALID_ARG, "%s",
                        _("at least 1 server is necessary in "
                          "JSON backing definition for gluster volume"));
diff --git a/tests/testutilsqemuschema.c b/tests/testutilsqemuschema.c
index 21f5d119e8..46bbc4f1e5 100644
--- a/tests/testutilsqemuschema.c
+++ b/tests/testutilsqemuschema.c
@@ -101,11 +101,9 @@ testQEMUSchemaStealObjectMemberByName(const char *name,
 {
     virJSONValuePtr member;
     virJSONValuePtr ret = NULL;
-    size_t n;
     size_t i;
 
-    n = virJSONValueArraySize(members);
-    for (i = 0; i < n; i++) {
+    for (i = 0; i < virJSONValueArraySize(members); i++) {
         member = virJSONValueArrayGet(members, i);
 
         if (STREQ_NULLABLE(name, virJSONValueObjectGetString(member, "name"))) {
@@ -188,7 +186,6 @@ testQEMUSchemaValidateObjectMergeVariant(virJSONValuePtr root,
                                          virHashTablePtr schema,
                                          virBufferPtr debug)
 {
-    size_t n;
     size_t i;
     virJSONValuePtr variants = NULL;
     virJSONValuePtr variant;
@@ -203,8 +200,7 @@ testQEMUSchemaValidateObjectMergeVariant(virJSONValuePtr root,
         return -2;
     }
 
-    n = virJSONValueArraySize(variants);
-    for (i = 0; i < n; i++) {
+    for (i = 0; i < virJSONValueArraySize(variants); i++) {
         variant = virJSONValueArrayGet(variants, i);
 
         if (STREQ_NULLABLE(variantname,
@@ -342,7 +338,6 @@ testQEMUSchemaValidateEnum(virJSONValuePtr obj,
     const char *objstr;
     virJSONValuePtr values = NULL;
     virJSONValuePtr value;
-    size_t n;
     size_t i;
 
     if (virJSONValueGetType(obj) != VIR_JSON_TYPE_STRING) {
@@ -358,8 +353,7 @@ testQEMUSchemaValidateEnum(virJSONValuePtr obj,
         return -2;
     }
 
-    n = virJSONValueArraySize(values);
-    for (i = 0; i < n; i++) {
+    for (i = 0; i < virJSONValueArraySize(values); i++) {
         value = virJSONValueArrayGet(values, i);
 
         if (STREQ_NULLABLE(objstr, virJSONValueGetString(value))) {
@@ -383,7 +377,6 @@ testQEMUSchemaValidateArray(virJSONValuePtr objs,
     const char *elemtypename = virJSONValueObjectGetString(root, "element-type");
     virJSONValuePtr elementschema;
     virJSONValuePtr obj;
-    size_t n;
     size_t i;
 
     if (virJSONValueGetType(objs) != VIR_JSON_TYPE_ARRAY) {
@@ -401,8 +394,7 @@ testQEMUSchemaValidateArray(virJSONValuePtr objs,
     virBufferAddLit(debug, "[\n");
     virBufferAdjustIndent(debug, 3);
 
-    n = virJSONValueArraySize(objs);
-    for (i = 0; i < n; i++) {
+    for (i = 0; i < virJSONValueArraySize(objs); i++) {
         obj = virJSONValueArrayGet(objs, i);
 
         if (testQEMUSchemaValidateRecurse(obj, elementschema, schema, debug) < 0)
@@ -423,8 +415,8 @@ testQEMUSchemaValidateAlternate(virJSONValuePtr obj,
 {
     virJSONValuePtr members;
     virJSONValuePtr member;
-    size_t n;
     size_t i;
+    size_t n;
     const char *membertype;
     virJSONValuePtr memberschema;
     int indent;
diff --git a/tools/nss/libvirt_nss.c b/tools/nss/libvirt_nss.c
index 62fe589bed..ec73ea575a 100644
--- a/tools/nss/libvirt_nss.c
+++ b/tools/nss/libvirt_nss.c
@@ -309,8 +309,7 @@ findLease(const char *name,
     }
     VIR_DIR_CLOSE(dir);
 
-    if ((nleases = virJSONValueArraySize(leases_array)) < 0)
-        goto cleanup;
+    nleases = virJSONValueArraySize(leases_array);
     DEBUG("Read %zd leases", nleases);
 
 #if !defined(LIBVIRT_NSS_GUEST)
-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [REPOST PATCH] util: Clean up consumers of virJSONValueArraySize
Posted by Ján Tomko 5 years, 10 months ago
On Thu, May 10, 2018 at 11:43:11AM -0400, John Ferlan wrote:
>Rather than have virJSONValueArraySize return a -1 when the input
>is not an array and then splat an error message, let's check for
>an array before calling and then change the return to be a size_t
>instead of ssize_t.
>
>That means using the helper virJSONValueIsArray as well as using a
>more generic error message such as "Malformed <something> array".
>In some cases we can remove stack variables and when we cannot,
>those variables should be size_t not ssize_t. Alter a few references
>of if (!value) to be if (value == 0) instead as well.
>
>Some callers can already assume an array is being worked on based
>on the previous call, so there's less to do.
>
>Signed-off-by: John Ferlan <jferlan@redhat.com>
>---
>
> Original:
> https://www.redhat.com/archives/libvir-list/2018-April/msg02003.html
>
> Reposting as I found a recent upstream changes created some simple
> merge conflicts and it's never clear if applying what was there can
> be applied on top even with git am -3, so I figured I would just repost.
>
> As noted previously, based off a review of the Coverity work from Peter:
>
> https://www.redhat.com/archives/libvir-list/2018-April/msg01613.html
>
> src/locking/lock_daemon.c     |  7 ++--
> src/logging/log_handler.c     |  7 ++--
> src/network/bridge_driver.c   |  7 ++--
> src/qemu/qemu_agent.c         | 35 ++++++++++--------
> src/qemu/qemu_monitor_json.c  | 85 ++++++++++++++++++++-----------------------
> src/rpc/virnetdaemon.c        |  7 +---
> src/rpc/virnetserver.c        | 15 +++-----
> src/rpc/virnetserverservice.c |  7 ++--
> src/util/virjson.c            |  5 +--
> src/util/virjson.h            |  2 +-
> src/util/virlockspace.c       | 16 ++++----
> src/util/virstoragefile.c     |  3 +-
> tests/testutilsqemuschema.c   | 18 +++------
> tools/nss/libvirt_nss.c       |  3 +-
> 14 files changed, 97 insertions(+), 120 deletions(-)
>
>@@ -1869,15 +1875,14 @@ qemuAgentGetFSInfo(qemuAgentPtr mon, virDomainFSInfoPtr **info,
>         goto cleanup;
>     }
>
>-    if (virJSONValueGetType(data) != VIR_JSON_TYPE_ARRAY) {
>+    if (!virJSONValueIsArray(data)) {
>         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>-                       _("guest-get-fsinfo return information was not "
>-                         "an array"));
>+                       _("Malformed guest-get-fsinfo data array"));
>         goto cleanup;
>     }
>
>     ndata = virJSONValueArraySize(data);
>-    if (!ndata) {
>+    if (ndata == 0) {
>         ret = 0;
>         *info = NULL;
>         goto cleanup;
>@@ -1928,14 +1933,14 @@ qemuAgentGetFSInfo(qemuAgentPtr mon, virDomainFSInfoPtr **info,
>             goto cleanup;
>         }
>
>-        if (virJSONValueGetType(entry) != VIR_JSON_TYPE_ARRAY) {
>+        if (!virJSONValueIsArray(entry)) {
>             virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>-                           _("guest-get-fsinfo 'disk' data was not an array"));
>+                           _("Malformed guest-get-fsinfo 'disk' data array"));
>             goto cleanup;
>         }
>
>         ndisk = virJSONValueArraySize(entry);
>-        if (!ndisk)
>+        if (ndisk == 0)
>             continue;
>         if (VIR_ALLOC_N(info_ret[i]->devAlias, ndisk) < 0)
>             goto cleanup;

Both ndata and ndisk should be switched to size_t in this function.

With that change:

Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list