Message ID | 20170824103350.16400-7-marcandre.lureau@redhat.com |
---|---|
State | New |
Headers | show |
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 --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); }
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(-)