diff mbox

[06/14] qlit: make qlit_equal_qobject return a bool

Message ID 20170824103350.16400-7-marcandre.lureau@redhat.com
State New
Headers show

Commit Message

Marc-André Lureau Aug. 24, 2017, 10:33 a.m. UTC
Make it more obvious about the expected return values.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/qapi/qmp/qlit.h |  2 +-
 qobject/qlit.c          | 18 +++++++++---------
 tests/check-qjson.c     | 14 +++++++-------
 3 files changed, 17 insertions(+), 17 deletions(-)

Comments

Markus Armbruster Aug. 25, 2017, 6:55 a.m. UTC | #1
Marc-André Lureau <marcandre.lureau@redhat.com> writes:

> Make it more obvious about the expected return values.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  include/qapi/qmp/qlit.h |  2 +-
>  qobject/qlit.c          | 18 +++++++++---------
>  tests/check-qjson.c     | 14 +++++++-------
>  3 files changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/include/qapi/qmp/qlit.h b/include/qapi/qmp/qlit.h
> index e299e8fab0..2e22d0f73c 100644
> --- a/include/qapi/qmp/qlit.h
> +++ b/include/qapi/qmp/qlit.h
> @@ -44,6 +44,6 @@ struct QLitDictEntry {
>  #define QLIT_QLIST(val) \
>      { .type = QTYPE_QLIST, .value.qlist = (val) }
>  
> -int qlit_equal_qobject(QLitObject *lhs, QObject *rhs);
> +bool qlit_equal_qobject(QLitObject *lhs, QObject *rhs);
>  
>  #endif /* QLIT_H_ */
> diff --git a/qobject/qlit.c b/qobject/qlit.c
> index 0c4101898d..a2975bef3f 100644
> --- a/qobject/qlit.c
> +++ b/qobject/qlit.c
> @@ -21,19 +21,19 @@
>  typedef struct QListCompareHelper {
>      int index;
>      QLitObject *objs;
> -    int result;
> +    bool result;
>  } QListCompareHelper;
>  
>  static void compare_helper(QObject *obj, void *opaque)
>  {
>      QListCompareHelper *helper = opaque;
>  
> -    if (helper->result == 0) {
> +    if (!helper->result) {
>          return;
>      }
>  
>      if (helper->objs[helper->index].type == QTYPE_NONE) {
> -        helper->result = 0;
> +        helper->result = false;
>          return;
>      }
>  
> @@ -41,12 +41,12 @@ static void compare_helper(QObject *obj, void *opaque)
>          qlit_equal_qobject(&helper->objs[helper->index++], obj);
>  }
>  
> -int qlit_equal_qobject(QLitObject *lhs, QObject *rhs)
> +bool qlit_equal_qobject(QLitObject *lhs, QObject *rhs)
>  {
>      int64_t val;
>  
>      if (!rhs || lhs->type != qobject_type(rhs)) {
> -        return 0;
> +        return false;
>      }
>  
>      switch (lhs->type) {
> @@ -64,18 +64,18 @@ int qlit_equal_qobject(QLitObject *lhs, QObject *rhs)
>                                       lhs->value.qdict[i].key);
>  
>              if (!qlit_equal_qobject(&lhs->value.qdict[i].value, obj)) {
> -                return 0;
> +                return false;
>              }
>          }
>  
> -        return 1;
> +        return true;
>      }
>      case QTYPE_QLIST: {
>          QListCompareHelper helper;
>  
>          helper.index = 0;
>          helper.objs = lhs->value.qlist;
> -        helper.result = 1;
> +        helper.result = true;
>  
>          qlist_iter(qobject_to_qlist(rhs), compare_helper, &helper);
>  

Let's use QLIST_FOREACH_ENTRY() rather than qlist_iter() with awkward
callback machinery.  Could be done before this patch (might reduce
churn) or after.  Even as a follow-up patch.

Speaking of additional cleanup: qlit_equal_dict() clones its QDict
argument for no good reason.  Drop the clone and compare
qdict_size(dict) == i instead.  Same result except when you got
duplicates in your QLitObject, but that's a silly programming error I
wouldn't bother to cope with or catch.

> @@ -85,5 +85,5 @@ int qlit_equal_qobject(QLitObject *lhs, QObject *rhs)
>          break;
>      }
>  
> -    return 0;
> +    return false;
>  }
> diff --git a/tests/check-qjson.c b/tests/check-qjson.c
> index e5ca273cbc..59227934ce 100644
> --- a/tests/check-qjson.c
> +++ b/tests/check-qjson.c
> @@ -1094,13 +1094,13 @@ static void simple_dict(void)
>          QString *str;
>  
>          obj = qobject_from_json(test_cases[i].encoded, &error_abort);
> -        g_assert(qlit_equal_qobject(&test_cases[i].decoded, obj) == 1);
> +        g_assert(qlit_equal_qobject(&test_cases[i].decoded, obj));
>  
>          str = qobject_to_json(obj);
>          qobject_decref(obj);
>  
>          obj = qobject_from_json(qstring_get_str(str), &error_abort);
> -        g_assert(qlit_equal_qobject(&test_cases[i].decoded, obj) == 1);
> +        g_assert(qlit_equal_qobject(&test_cases[i].decoded, obj));
>          qobject_decref(obj);
>          QDECREF(str);
>      }
> @@ -1203,13 +1203,13 @@ static void simple_list(void)
>          QString *str;
>  
>          obj = qobject_from_json(test_cases[i].encoded, &error_abort);
> -        g_assert(qlit_equal_qobject(&test_cases[i].decoded, obj) == 1);
> +        g_assert(qlit_equal_qobject(&test_cases[i].decoded, obj));
>  
>          str = qobject_to_json(obj);
>          qobject_decref(obj);
>  
>          obj = qobject_from_json(qstring_get_str(str), &error_abort);
> -        g_assert(qlit_equal_qobject(&test_cases[i].decoded, obj) == 1);
> +        g_assert(qlit_equal_qobject(&test_cases[i].decoded, obj));
>          qobject_decref(obj);
>          QDECREF(str);
>      }
> @@ -1265,13 +1265,13 @@ static void simple_whitespace(void)
>          QString *str;
>  
>          obj = qobject_from_json(test_cases[i].encoded, &error_abort);
> -        g_assert(qlit_equal_qobject(&test_cases[i].decoded, obj) == 1);
> +        g_assert(qlit_equal_qobject(&test_cases[i].decoded, obj));
>  
>          str = qobject_to_json(obj);
>          qobject_decref(obj);
>  
>          obj = qobject_from_json(qstring_get_str(str), &error_abort);
> -        g_assert(qlit_equal_qobject(&test_cases[i].decoded, obj) == 1);
> +        g_assert(qlit_equal_qobject(&test_cases[i].decoded, obj));
>  
>          qobject_decref(obj);
>          QDECREF(str);
> @@ -1295,7 +1295,7 @@ static void simple_varargs(void)
>      g_assert(embedded_obj != NULL);
>  
>      obj = qobject_from_jsonf("[%d, 2, %p]", 1, embedded_obj);
> -    g_assert(qlit_equal_qobject(&decoded, obj) == 1);
> +    g_assert(qlit_equal_qobject(&decoded, obj));
>  
>      qobject_decref(obj);
>  }

Reviewed-by: Markus Armbruster <armbru@redhat.com>

I encourage you to pursue the additional cleanups I outlined above.
diff mbox

Patch

diff --git a/include/qapi/qmp/qlit.h b/include/qapi/qmp/qlit.h
index e299e8fab0..2e22d0f73c 100644
--- a/include/qapi/qmp/qlit.h
+++ b/include/qapi/qmp/qlit.h
@@ -44,6 +44,6 @@  struct QLitDictEntry {
 #define QLIT_QLIST(val) \
     { .type = QTYPE_QLIST, .value.qlist = (val) }
 
-int qlit_equal_qobject(QLitObject *lhs, QObject *rhs);
+bool qlit_equal_qobject(QLitObject *lhs, QObject *rhs);
 
 #endif /* QLIT_H_ */
diff --git a/qobject/qlit.c b/qobject/qlit.c
index 0c4101898d..a2975bef3f 100644
--- a/qobject/qlit.c
+++ b/qobject/qlit.c
@@ -21,19 +21,19 @@ 
 typedef struct QListCompareHelper {
     int index;
     QLitObject *objs;
-    int result;
+    bool result;
 } QListCompareHelper;
 
 static void compare_helper(QObject *obj, void *opaque)
 {
     QListCompareHelper *helper = opaque;
 
-    if (helper->result == 0) {
+    if (!helper->result) {
         return;
     }
 
     if (helper->objs[helper->index].type == QTYPE_NONE) {
-        helper->result = 0;
+        helper->result = false;
         return;
     }
 
@@ -41,12 +41,12 @@  static void compare_helper(QObject *obj, void *opaque)
         qlit_equal_qobject(&helper->objs[helper->index++], obj);
 }
 
-int qlit_equal_qobject(QLitObject *lhs, QObject *rhs)
+bool qlit_equal_qobject(QLitObject *lhs, QObject *rhs)
 {
     int64_t val;
 
     if (!rhs || lhs->type != qobject_type(rhs)) {
-        return 0;
+        return false;
     }
 
     switch (lhs->type) {
@@ -64,18 +64,18 @@  int qlit_equal_qobject(QLitObject *lhs, QObject *rhs)
                                      lhs->value.qdict[i].key);
 
             if (!qlit_equal_qobject(&lhs->value.qdict[i].value, obj)) {
-                return 0;
+                return false;
             }
         }
 
-        return 1;
+        return true;
     }
     case QTYPE_QLIST: {
         QListCompareHelper helper;
 
         helper.index = 0;
         helper.objs = lhs->value.qlist;
-        helper.result = 1;
+        helper.result = true;
 
         qlist_iter(qobject_to_qlist(rhs), compare_helper, &helper);
 
@@ -85,5 +85,5 @@  int qlit_equal_qobject(QLitObject *lhs, QObject *rhs)
         break;
     }
 
-    return 0;
+    return false;
 }
diff --git a/tests/check-qjson.c b/tests/check-qjson.c
index e5ca273cbc..59227934ce 100644
--- a/tests/check-qjson.c
+++ b/tests/check-qjson.c
@@ -1094,13 +1094,13 @@  static void simple_dict(void)
         QString *str;
 
         obj = qobject_from_json(test_cases[i].encoded, &error_abort);
-        g_assert(qlit_equal_qobject(&test_cases[i].decoded, obj) == 1);
+        g_assert(qlit_equal_qobject(&test_cases[i].decoded, obj));
 
         str = qobject_to_json(obj);
         qobject_decref(obj);
 
         obj = qobject_from_json(qstring_get_str(str), &error_abort);
-        g_assert(qlit_equal_qobject(&test_cases[i].decoded, obj) == 1);
+        g_assert(qlit_equal_qobject(&test_cases[i].decoded, obj));
         qobject_decref(obj);
         QDECREF(str);
     }
@@ -1203,13 +1203,13 @@  static void simple_list(void)
         QString *str;
 
         obj = qobject_from_json(test_cases[i].encoded, &error_abort);
-        g_assert(qlit_equal_qobject(&test_cases[i].decoded, obj) == 1);
+        g_assert(qlit_equal_qobject(&test_cases[i].decoded, obj));
 
         str = qobject_to_json(obj);
         qobject_decref(obj);
 
         obj = qobject_from_json(qstring_get_str(str), &error_abort);
-        g_assert(qlit_equal_qobject(&test_cases[i].decoded, obj) == 1);
+        g_assert(qlit_equal_qobject(&test_cases[i].decoded, obj));
         qobject_decref(obj);
         QDECREF(str);
     }
@@ -1265,13 +1265,13 @@  static void simple_whitespace(void)
         QString *str;
 
         obj = qobject_from_json(test_cases[i].encoded, &error_abort);
-        g_assert(qlit_equal_qobject(&test_cases[i].decoded, obj) == 1);
+        g_assert(qlit_equal_qobject(&test_cases[i].decoded, obj));
 
         str = qobject_to_json(obj);
         qobject_decref(obj);
 
         obj = qobject_from_json(qstring_get_str(str), &error_abort);
-        g_assert(qlit_equal_qobject(&test_cases[i].decoded, obj) == 1);
+        g_assert(qlit_equal_qobject(&test_cases[i].decoded, obj));
 
         qobject_decref(obj);
         QDECREF(str);
@@ -1295,7 +1295,7 @@  static void simple_varargs(void)
     g_assert(embedded_obj != NULL);
 
     obj = qobject_from_jsonf("[%d, 2, %p]", 1, embedded_obj);
-    g_assert(qlit_equal_qobject(&decoded, obj) == 1);
+    g_assert(qlit_equal_qobject(&decoded, obj));
 
     qobject_decref(obj);
 }