[PATCH] virjson: Change virJSONValueObjectHasKey() signature

Michal Privoznik posted 1 patch 1 year, 8 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/79193fc94035ccd84ae6848cfedc37fe578eecd3.1659015213.git.mprivozn@redhat.com
src/qemu/qemu_agent.c        |  8 ++++----
src/qemu/qemu_monitor_json.c | 10 +++++-----
src/util/virjson.c           |  8 ++++----
src/util/virjson.h           |  2 +-
4 files changed, 14 insertions(+), 14 deletions(-)
[PATCH] virjson: Change virJSONValueObjectHasKey() signature
Posted by Michal Privoznik 1 year, 8 months ago
Currently, virJSONValueObjectHasKey() can return one of three
values:

  -1 if passed object type is not VIR_JSON_TYPE_OBJECT,
   0 if the key is not present, and finally
   1 if the key is present.

But, neither of callers is interested in the -1 case. In fact,
some callers call this function treating -1 and 1 cases the same.
Therefore, make the function return just true/false and fix few
callers that explicitly checked for == 1 case.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/qemu/qemu_agent.c        |  8 ++++----
 src/qemu/qemu_monitor_json.c | 10 +++++-----
 src/util/virjson.c           |  8 ++++----
 src/util/virjson.h           |  2 +-
 4 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index d81f01ba77..0d77a2f90d 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -231,12 +231,12 @@ qemuAgentIOProcessLine(qemuAgent *agent,
         return -1;
     }
 
-    if (virJSONValueObjectHasKey(obj, "QMP") == 1) {
+    if (virJSONValueObjectHasKey(obj, "QMP")) {
         return 0;
-    } else if (virJSONValueObjectHasKey(obj, "event") == 1) {
+    } else if (virJSONValueObjectHasKey(obj, "event")) {
         return qemuAgentIOProcessEvent(agent, obj);
-    } else if (virJSONValueObjectHasKey(obj, "error") == 1 ||
-               virJSONValueObjectHasKey(obj, "return") == 1) {
+    } else if (virJSONValueObjectHasKey(obj, "error") ||
+               virJSONValueObjectHasKey(obj, "return")) {
         if (msg) {
             if (msg->sync) {
                 unsigned long long id;
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 941596563a..2469165728 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -203,14 +203,14 @@ qemuMonitorJSONIOProcessLine(qemuMonitor *mon,
         return -1;
     }
 
-    if (virJSONValueObjectHasKey(obj, "QMP") == 1) {
+    if (virJSONValueObjectHasKey(obj, "QMP")) {
         return 0;
-    } else if (virJSONValueObjectHasKey(obj, "event") == 1) {
+    } else if (virJSONValueObjectHasKey(obj, "event")) {
         PROBE(QEMU_MONITOR_RECV_EVENT,
               "mon=%p event=%s", mon, line);
         return qemuMonitorJSONIOProcessEvent(mon, obj);
-    } else if (virJSONValueObjectHasKey(obj, "error") == 1 ||
-               virJSONValueObjectHasKey(obj, "return") == 1) {
+    } else if (virJSONValueObjectHasKey(obj, "error") ||
+               virJSONValueObjectHasKey(obj, "return")) {
         PROBE(QEMU_MONITOR_RECV_REPLY,
               "mon=%p reply=%s", mon, line);
         if (msg) {
@@ -270,7 +270,7 @@ qemuMonitorJSONCommandWithFd(qemuMonitor *mon,
 
     memset(&msg, 0, sizeof(msg));
 
-    if (virJSONValueObjectHasKey(cmd, "execute") == 1) {
+    if (virJSONValueObjectHasKey(cmd, "execute")) {
         g_autofree char *id = qemuMonitorNextCommandID(mon);
 
         if (virJSONValueObjectAppendString(cmd, "id", id) < 0) {
diff --git a/src/util/virjson.c b/src/util/virjson.c
index 53f8cdff95..ef59b5deb4 100644
--- a/src/util/virjson.c
+++ b/src/util/virjson.c
@@ -798,21 +798,21 @@ virJSONValueArrayConcat(virJSONValue *a,
 }
 
 
-int
+bool
 virJSONValueObjectHasKey(virJSONValue *object,
                          const char *key)
 {
     size_t i;
 
     if (object->type != VIR_JSON_TYPE_OBJECT)
-        return -1;
+        return false;
 
     for (i = 0; i < object->data.object.npairs; i++) {
         if (STREQ(object->data.object.pairs[i].key, key))
-            return 1;
+            return true;
     }
 
-    return 0;
+    return false;
 }
 
 
diff --git a/src/util/virjson.h b/src/util/virjson.h
index aced48a538..ce181dcb82 100644
--- a/src/util/virjson.h
+++ b/src/util/virjson.h
@@ -88,7 +88,7 @@ int
 virJSONValueArrayConcat(virJSONValue *a,
                         virJSONValue *c);
 
-int
+bool
 virJSONValueObjectHasKey(virJSONValue *object,
                          const char *key);
 virJSONValue *
-- 
2.35.1
Re: [PATCH] virjson: Change virJSONValueObjectHasKey() signature
Posted by Peter Krempa 1 year, 8 months ago
On Thu, Jul 28, 2022 at 15:33:33 +0200, Michal Privoznik wrote:
> Currently, virJSONValueObjectHasKey() can return one of three
> values:
> 
>   -1 if passed object type is not VIR_JSON_TYPE_OBJECT,
>    0 if the key is not present, and finally
>    1 if the key is present.
> 
> But, neither of callers is interested in the -1 case. In fact,
> some callers call this function treating -1 and 1 cases the same.
> Therefore, make the function return just true/false and fix few
> callers that explicitly checked for == 1 case.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/qemu/qemu_agent.c        |  8 ++++----
>  src/qemu/qemu_monitor_json.c | 10 +++++-----
>  src/util/virjson.c           |  8 ++++----
>  src/util/virjson.h           |  2 +-
>  4 files changed, 14 insertions(+), 14 deletions(-)

You forgot to fix one instance in testQEMUSchemaValidateObjectMandatoryMember
which uses '!= 1' instead of '== 1'.

With that fixed:

Reviewed-by: Peter Krempa <pkrempa@redhat.com>