The code that processes list of device properties is going to be
reused. Therefore put it into a separate function.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/qemu/qemu_monitor_json.c | 59 +++++++++++++++++++++++++++-----------------
 1 file changed, 36 insertions(+), 23 deletions(-)

diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 24d37eb41d..95b9d60aff 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -6053,35 +6053,19 @@ int qemuMonitorJSONSetObjectProperty(qemuMonitorPtr mon,
 #undef MAKE_SET_CMD
 
 
-int qemuMonitorJSONGetDeviceProps(qemuMonitorPtr mon,
-                                  const char *device,
-                                  char ***props)
+static int
+qemuMonitorJSONParsePropsList(virJSONValuePtr cmd,
+                              virJSONValuePtr reply,
+                              char ***props)
 {
-    int ret = -1;
-    virJSONValuePtr cmd;
-    virJSONValuePtr reply = NULL;
     virJSONValuePtr data;
     char **proplist = NULL;
     ssize_t n = 0;
     size_t i;
-
-    *props = NULL;
-
-    if (!(cmd = qemuMonitorJSONMakeCommand("device-list-properties",
-                                           "s:typename", device,
-                                           NULL)))
-        return -1;
-
-    if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
-        goto cleanup;
-
-    if (qemuMonitorJSONHasError(reply, "DeviceNotFound")) {
-        ret = 0;
-        goto cleanup;
-    }
+    int ret = -1;
 
     if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_ARRAY) < 0)
-        goto cleanup;
+        return ret;
 
     data = virJSONValueObjectGetArray(reply, "return");
     n = virJSONValueArraySize(data);
@@ -6110,8 +6094,37 @@ int qemuMonitorJSONGetDeviceProps(qemuMonitorPtr mon,
 
  cleanup:
     virStringListFree(proplist);
-    virJSONValueFree(cmd);
+    return ret;
+}
+
+
+int qemuMonitorJSONGetDeviceProps(qemuMonitorPtr mon,
+                                  const char *device,
+                                  char ***props)
+{
+    int ret = -1;
+    virJSONValuePtr cmd;
+    virJSONValuePtr reply = NULL;
+
+    *props = NULL;
+
+    if (!(cmd = qemuMonitorJSONMakeCommand("device-list-properties",
+                                           "s:typename", device,
+                                           NULL)))
+        return -1;
+
+    if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
+        goto cleanup;
+
+    if (qemuMonitorJSONHasError(reply, "DeviceNotFound")) {
+        ret = 0;
+        goto cleanup;
+    }
+
+    ret = qemuMonitorJSONParsePropsList(cmd, reply, props);
+ cleanup:
     virJSONValueFree(reply);
+    virJSONValueFree(cmd);
     return ret;
 }
 
-- 
2.16.1

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

On Fri, Apr 20, 2018 at 11:09:27AM +0200, Michal Privoznik wrote:
>The code that processes list of device properties is going to be
>reused. Therefore put it into a separate function.
>
>Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>---
> src/qemu/qemu_monitor_json.c | 59 +++++++++++++++++++++++++++-----------------
> 1 file changed, 36 insertions(+), 23 deletions(-)
>
>diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
>index 24d37eb41d..95b9d60aff 100644
>--- a/src/qemu/qemu_monitor_json.c
>+++ b/src/qemu/qemu_monitor_json.c
>@@ -6053,35 +6053,19 @@ int qemuMonitorJSONSetObjectProperty(qemuMonitorPtr mon,
> #undef MAKE_SET_CMD
>
>
>-int qemuMonitorJSONGetDeviceProps(qemuMonitorPtr mon,
>-                                  const char *device,
>-                                  char ***props)
>+static int
>+qemuMonitorJSONParsePropsList(virJSONValuePtr cmd,
>+                              virJSONValuePtr reply,
>+                              char ***props)
> {
>-    int ret = -1;
>-    virJSONValuePtr cmd;
>-    virJSONValuePtr reply = NULL;
>     virJSONValuePtr data;
>     char **proplist = NULL;
>     ssize_t n = 0;
>     size_t i;
>-
>-    *props = NULL;
>-
>-    if (!(cmd = qemuMonitorJSONMakeCommand("device-list-properties",
>-                                           "s:typename", device,
>-                                           NULL)))
>-        return -1;
>-
>-    if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
>-        goto cleanup;
>-
>-    if (qemuMonitorJSONHasError(reply, "DeviceNotFound")) {
>-        ret = 0;
>-        goto cleanup;
>-    }
>+    int ret = -1;
>
>     if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_ARRAY) < 0)
>-        goto cleanup;
>+        return ret;

Here I'd prefer either 'return -1' or 'goto cleanup', to avoid the
impression that the value in 'ret' can possibly be significant here.

More importantly, there is another mention of device-list-properties in
the error message below:

if (!(tmp = virJSONValueObjectGetString(child, "name"))) {
    virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                   _("device-list-properties reply data was missing 'name'"));
    goto cleanup;
}

Personally, I'd just s/device-list-properties //, because the error is
unlikely.

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