[PATCH for-5.1 0/5] qobject: Minor spring cleaning

[PATCH for-5.1 0/5] qobject: Minor spring cleaning

[Markus Armbruster]

Markus Armbruster (5):
  qobject: Clean up QLIST_FOREACH_ENTRY()
  qobject: Factor out helper json_pretty_newline()
  qobject: Eliminate qlist_iter(), use QLIST_FOREACH_ENTRY() instead
  qobject: Eliminate qdict_iter(), use qdict_first(), qdict_next()
  qemu-option: Clean up after the previous commit

 include/qapi/qmp/qdict.h     |   3 -
 include/qapi/qmp/qlist.h     |  10 ++--
 qapi/qobject-input-visitor.c |  21 +++----
 qobject/qdict.c              |  19 -------
 qobject/qjson.c              | 107 +++++++++++++----------------------
 qobject/qlist.c              |  44 ++++----------
 tests/check-qlist.c          |  37 +++++-------
 util/qemu-option.c           |  43 +++++++-------
 8 files changed, 98 insertions(+), 186 deletions(-)

-- 
2.21.1

[PATCH for-5.1 1/5] qobject: Clean up QLIST_FOREACH_ENTRY()

[Markus Armbruster]

QLIST_FOREACH_ENTRY() traverses a tail queue manually.  Use
QTAILQ_FIRST() and QTAILQ_NEXT() instead.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/qapi/qmp/qlist.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/qapi/qmp/qlist.h b/include/qapi/qmp/qlist.h
index 8d2c32ca28..07ecae81e4 100644
--- a/include/qapi/qmp/qlist.h
+++ b/include/qapi/qmp/qlist.h
@@ -34,10 +34,10 @@ void qlist_append_int(QList *qlist, int64_t value);
 void qlist_append_null(QList *qlist);
 void qlist_append_str(QList *qlist, const char *value);
 
-#define QLIST_FOREACH_ENTRY(qlist, var)             \
-        for ((var) = ((qlist)->head.tqh_first);     \
-            (var);                                  \
-            (var) = ((var)->next.tqe_next))
+#define QLIST_FOREACH_ENTRY(qlist, var)                 \
+        for ((var) = QTAILQ_FIRST(&(qlist)->head);      \
+             (var);                                     \
+             (var) = QTAILQ_NEXT((var), next))
 
 static inline QObject *qlist_entry_obj(const QListEntry *entry)
 {
-- 
2.21.1

[PATCH for-5.1 2/5] qobject: Factor out helper json_pretty_newline()

[Markus Armbruster]

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qobject/qjson.c | 40 ++++++++++++++++------------------------
 1 file changed, 16 insertions(+), 24 deletions(-)

diff --git a/qobject/qjson.c b/qobject/qjson.c
index db36101f3b..f3c62711b9 100644
--- a/qobject/qjson.c
+++ b/qobject/qjson.c
@@ -159,21 +159,28 @@ typedef struct ToJsonIterState
 
 static void to_json(const QObject *obj, QString *str, int pretty, int indent);
 
+static void json_pretty_newline(QString *str, bool pretty, int indent)
+{
+    int i;
+
+    if (pretty) {
+        qstring_append(str, "\n");
+        for (i = 0 ; i < indent ; i++) {
+            qstring_append(str, "    ");
+        }
+    }
+}
+
 static void to_json_dict_iter(const char *key, QObject *obj, void *opaque)
 {
     ToJsonIterState *s = opaque;
     QString *qkey;
-    int j;
 
     if (s->count) {
         qstring_append(s->str, s->pretty ? "," : ", ");
     }
 
-    if (s->pretty) {
-        qstring_append(s->str, "\n");
-        for (j = 0 ; j < s->indent ; j++)
-            qstring_append(s->str, "    ");
-    }
+    json_pretty_newline(s->str, s->pretty, s->indent);
 
     qkey = qstring_from_str(key);
     to_json(QOBJECT(qkey), s->str, s->pretty, s->indent);
@@ -187,17 +194,12 @@ static void to_json_dict_iter(const char *key, QObject *obj, void *opaque)
 static void to_json_list_iter(QObject *obj, void *opaque)
 {
     ToJsonIterState *s = opaque;
-    int j;
 
     if (s->count) {
         qstring_append(s->str, s->pretty ? "," : ", ");
     }
 
-    if (s->pretty) {
-        qstring_append(s->str, "\n");
-        for (j = 0 ; j < s->indent ; j++)
-            qstring_append(s->str, "    ");
-    }
+    json_pretty_newline(s->str, s->pretty, s->indent);
 
     to_json(obj, s->str, s->pretty, s->indent);
     s->count++;
@@ -282,12 +284,7 @@ static void to_json(const QObject *obj, QString *str, int pretty, int indent)
         s.pretty = pretty;
         qstring_append(str, "{");
         qdict_iter(val, to_json_dict_iter, &s);
-        if (pretty) {
-            int j;
-            qstring_append(str, "\n");
-            for (j = 0 ; j < indent ; j++)
-                qstring_append(str, "    ");
-        }
+        json_pretty_newline(str, pretty, indent);
         qstring_append(str, "}");
         break;
     }
@@ -301,12 +298,7 @@ static void to_json(const QObject *obj, QString *str, int pretty, int indent)
         s.pretty = pretty;
         qstring_append(str, "[");
         qlist_iter(val, (void *)to_json_list_iter, &s);
-        if (pretty) {
-            int j;
-            qstring_append(str, "\n");
-            for (j = 0 ; j < indent ; j++)
-                qstring_append(str, "    ");
-        }
+        json_pretty_newline(str, pretty, indent);
         qstring_append(str, "]");
         break;
     }
-- 
2.21.1

[PATCH for-5.1 3/5] qobject: Eliminate qlist_iter(), use QLIST_FOREACH_ENTRY() instead

[Markus Armbruster]

qlist_iter() has just three uses outside tests/.  Replace by
QLIST_FOREACH_ENTRY() for more concise code and less type punning.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/qapi/qmp/qlist.h |  2 --
 qobject/qjson.c          | 31 ++++++++++------------------
 qobject/qlist.c          | 44 +++++++++++-----------------------------
 tests/check-qlist.c      | 37 +++++++++++++--------------------
 4 files changed, 37 insertions(+), 77 deletions(-)

diff --git a/include/qapi/qmp/qlist.h b/include/qapi/qmp/qlist.h
index 07ecae81e4..595b7943e1 100644
--- a/include/qapi/qmp/qlist.h
+++ b/include/qapi/qmp/qlist.h
@@ -47,8 +47,6 @@ static inline QObject *qlist_entry_obj(const QListEntry *entry)
 QList *qlist_new(void);
 QList *qlist_copy(QList *src);
 void qlist_append_obj(QList *qlist, QObject *obj);
-void qlist_iter(const QList *qlist,
-                void (*iter)(QObject *obj, void *opaque), void *opaque);
 QObject *qlist_pop(QList *qlist);
 QObject *qlist_peek(QList *qlist);
 int qlist_empty(const QList *qlist);
diff --git a/qobject/qjson.c b/qobject/qjson.c
index f3c62711b9..10050354a0 100644
--- a/qobject/qjson.c
+++ b/qobject/qjson.c
@@ -191,20 +191,6 @@ static void to_json_dict_iter(const char *key, QObject *obj, void *opaque)
     s->count++;
 }
 
-static void to_json_list_iter(QObject *obj, void *opaque)
-{
-    ToJsonIterState *s = opaque;
-
-    if (s->count) {
-        qstring_append(s->str, s->pretty ? "," : ", ");
-    }
-
-    json_pretty_newline(s->str, s->pretty, s->indent);
-
-    to_json(obj, s->str, s->pretty, s->indent);
-    s->count++;
-}
-
 static void to_json(const QObject *obj, QString *str, int pretty, int indent)
 {
     switch (qobject_type(obj)) {
@@ -289,15 +275,20 @@ static void to_json(const QObject *obj, QString *str, int pretty, int indent)
         break;
     }
     case QTYPE_QLIST: {
-        ToJsonIterState s;
         QList *val = qobject_to(QList, obj);
+        const char *comma = pretty ? "," : ", ";
+        const char *sep = "";
+        QListEntry *entry;
 
-        s.count = 0;
-        s.str = str;
-        s.indent = indent + 1;
-        s.pretty = pretty;
         qstring_append(str, "[");
-        qlist_iter(val, (void *)to_json_list_iter, &s);
+
+        QLIST_FOREACH_ENTRY(val, entry) {
+            qstring_append(str, sep);
+            json_pretty_newline(str, pretty, indent + 1);
+            to_json(qlist_entry_obj(entry), str, pretty, indent + 1);
+            sep = comma;
+        }
+
         json_pretty_newline(str, pretty, indent);
         qstring_append(str, "]");
         break;
diff --git a/qobject/qlist.c b/qobject/qlist.c
index b3274af88b..1be95367d1 100644
--- a/qobject/qlist.c
+++ b/qobject/qlist.c
@@ -34,20 +34,17 @@ QList *qlist_new(void)
     return qlist;
 }
 
-static void qlist_copy_elem(QObject *obj, void *opaque)
-{
-    QList *dst = opaque;
-
-    qobject_ref(obj);
-    qlist_append_obj(dst, obj);
-}
-
 QList *qlist_copy(QList *src)
 {
     QList *dst = qlist_new();
+    QListEntry *entry;
+    QObject *elt;
 
-    qlist_iter(src, qlist_copy_elem, dst);
-
+    QLIST_FOREACH_ENTRY(src, entry) {
+        elt = qlist_entry_obj(entry);
+        qobject_ref(elt);
+        qlist_append_obj(dst, elt);
+    }
     return dst;
 }
 
@@ -86,21 +83,6 @@ void qlist_append_null(QList *qlist)
     qlist_append(qlist, qnull());
 }
 
-/**
- * qlist_iter(): Iterate over all the list's stored values.
- *
- * This function allows the user to provide an iterator, which will be
- * called for each stored value in the list.
- */
-void qlist_iter(const QList *qlist,
-                void (*iter)(QObject *obj, void *opaque), void *opaque)
-{
-    QListEntry *entry;
-
-    QTAILQ_FOREACH(entry, &qlist->head, next)
-        iter(entry->value, opaque);
-}
-
 QObject *qlist_pop(QList *qlist)
 {
     QListEntry *entry;
@@ -137,16 +119,14 @@ int qlist_empty(const QList *qlist)
     return QTAILQ_EMPTY(&qlist->head);
 }
 
-static void qlist_size_iter(QObject *obj, void *opaque)
-{
-    size_t *count = opaque;
-    (*count)++;
-}
-
 size_t qlist_size(const QList *qlist)
 {
     size_t count = 0;
-    qlist_iter(qlist, qlist_size_iter, &count);
+    QListEntry *entry;
+
+    QLIST_FOREACH_ENTRY(qlist, entry) {
+        count++;
+    }
     return count;
 }
 
diff --git a/tests/check-qlist.c b/tests/check-qlist.c
index ece83e293d..3cd0ccbf19 100644
--- a/tests/check-qlist.c
+++ b/tests/check-qlist.c
@@ -61,40 +61,31 @@ static void qobject_to_qlist_test(void)
     qobject_unref(qlist);
 }
 
-static int iter_called;
-static const int iter_max = 42;
-
-static void iter_func(QObject *obj, void *opaque)
-{
-    QNum *qi;
-    int64_t val;
-
-    g_assert(opaque == NULL);
-
-    qi = qobject_to(QNum, obj);
-    g_assert(qi != NULL);
-
-    g_assert(qnum_get_try_int(qi, &val));
-    g_assert_cmpint(val, >=, 0);
-    g_assert_cmpint(val, <=, iter_max);
-
-    iter_called++;
-}
-
 static void qlist_iter_test(void)
 {
+    const int iter_max = 42;
     int i;
     QList *qlist;
+    QListEntry *entry;
+    QNum *qi;
+    int64_t val;
 
     qlist = qlist_new();
 
     for (i = 0; i < iter_max; i++)
         qlist_append_int(qlist, i);
 
-    iter_called = 0;
-    qlist_iter(qlist, iter_func, NULL);
+    i = 0;
+    QLIST_FOREACH_ENTRY(qlist, entry) {
+        qi = qobject_to(QNum, qlist_entry_obj(entry));
+        g_assert(qi != NULL);
 
-    g_assert(iter_called == iter_max);
+        g_assert(qnum_get_try_int(qi, &val));
+        g_assert_cmpint(val, ==, i);
+        i++;
+    }
+
+    g_assert(i == iter_max);
 
     qobject_unref(qlist);
 }
-- 
2.21.1

[PATCH for-5.1 4/5] qobject: Eliminate qdict_iter(), use qdict_first(), qdict_next()

[Markus Armbruster]

qdict_iter() has just three uses and no test coverage.  Replace by
qdict_first(), qdict_next() for more concise code and less type
punning.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/qapi/qmp/qdict.h     |  3 --
 qapi/qobject-input-visitor.c | 21 +++++++-------
 qobject/qdict.c              | 19 -------------
 qobject/qjson.c              | 54 +++++++++++++-----------------------
 util/qemu-option.c           | 10 ++++++-
 5 files changed, 40 insertions(+), 67 deletions(-)

diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h
index 7f3ec10a10..da942347a7 100644
--- a/include/qapi/qmp/qdict.h
+++ b/include/qapi/qmp/qdict.h
@@ -40,9 +40,6 @@ void qdict_del(QDict *qdict, const char *key);
 int qdict_haskey(const QDict *qdict, const char *key);
 QObject *qdict_get(const QDict *qdict, const char *key);
 bool qdict_is_equal(const QObject *x, const QObject *y);
-void qdict_iter(const QDict *qdict,
-                void (*iter)(const char *key, QObject *obj, void *opaque),
-                void *opaque);
 const QDictEntry *qdict_first(const QDict *qdict);
 const QDictEntry *qdict_next(const QDict *qdict, const QDictEntry *entry);
 void qdict_destroy_obj(QObject *obj);
diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
index 32236cbcb1..5ce3ec2e5f 100644
--- a/qapi/qobject-input-visitor.c
+++ b/qapi/qobject-input-visitor.c
@@ -203,31 +203,32 @@ static const char *qobject_input_get_keyval(QObjectInputVisitor *qiv,
     return qstring_get_str(qstr);
 }
 
-static void qdict_add_key(const char *key, QObject *obj, void *opaque)
-{
-    GHashTable *h = opaque;
-    g_hash_table_insert(h, (gpointer) key, NULL);
-}
-
 static const QListEntry *qobject_input_push(QObjectInputVisitor *qiv,
                                             const char *name,
                                             QObject *obj, void *qapi)
 {
     GHashTable *h;
     StackObject *tos = g_new0(StackObject, 1);
+    QDict *qdict = qobject_to(QDict, obj);
+    QList *qlist = qobject_to(QList, obj);
+    const QDictEntry *entry;
 
     assert(obj);
     tos->name = name;
     tos->obj = obj;
     tos->qapi = qapi;
 
-    if (qobject_type(obj) == QTYPE_QDICT) {
+    if (qdict) {
         h = g_hash_table_new(g_str_hash, g_str_equal);
-        qdict_iter(qobject_to(QDict, obj), qdict_add_key, h);
+        for (entry = qdict_first(qdict);
+             entry;
+             entry = qdict_next(qdict, entry)) {
+            g_hash_table_insert(h, (void *)qdict_entry_key(entry), NULL);
+        }
         tos->h = h;
     } else {
-        assert(qobject_type(obj) == QTYPE_QLIST);
-        tos->entry = qlist_first(qobject_to(QList, obj));
+        assert(qlist);
+        tos->entry = qlist_first(qlist);
         tos->index = -1;
     }
 
diff --git a/qobject/qdict.c b/qobject/qdict.c
index 3d8c2f7bbc..526de54ceb 100644
--- a/qobject/qdict.c
+++ b/qobject/qdict.c
@@ -298,25 +298,6 @@ const char *qdict_get_try_str(const QDict *qdict, const char *key)
     return qstr ? qstring_get_str(qstr) : NULL;
 }
 
-/**
- * qdict_iter(): Iterate over all the dictionary's stored values.
- *
- * This function allows the user to provide an iterator, which will be
- * called for each stored value in the dictionary.
- */
-void qdict_iter(const QDict *qdict,
-                void (*iter)(const char *key, QObject *obj, void *opaque),
-                void *opaque)
-{
-    int i;
-    QDictEntry *entry;
-
-    for (i = 0; i < QDICT_BUCKET_MAX; i++) {
-        QLIST_FOREACH(entry, &qdict->table[i], next)
-            iter(entry->key, entry->value, opaque);
-    }
-}
-
 static QDictEntry *qdict_next_entry(const QDict *qdict, int first_bucket)
 {
     int i;
diff --git a/qobject/qjson.c b/qobject/qjson.c
index 10050354a0..2a3336e459 100644
--- a/qobject/qjson.c
+++ b/qobject/qjson.c
@@ -149,14 +149,6 @@ QDict *qdict_from_jsonf_nofail(const char *string, ...)
     return qdict;
 }
 
-typedef struct ToJsonIterState
-{
-    int indent;
-    int pretty;
-    int count;
-    QString *str;
-} ToJsonIterState;
-
 static void to_json(const QObject *obj, QString *str, int pretty, int indent);
 
 static void json_pretty_newline(QString *str, bool pretty, int indent)
@@ -171,26 +163,6 @@ static void json_pretty_newline(QString *str, bool pretty, int indent)
     }
 }
 
-static void to_json_dict_iter(const char *key, QObject *obj, void *opaque)
-{
-    ToJsonIterState *s = opaque;
-    QString *qkey;
-
-    if (s->count) {
-        qstring_append(s->str, s->pretty ? "," : ", ");
-    }
-
-    json_pretty_newline(s->str, s->pretty, s->indent);
-
-    qkey = qstring_from_str(key);
-    to_json(QOBJECT(qkey), s->str, s->pretty, s->indent);
-    qobject_unref(qkey);
-
-    qstring_append(s->str, ": ");
-    to_json(obj, s->str, s->pretty, s->indent);
-    s->count++;
-}
-
 static void to_json(const QObject *obj, QString *str, int pretty, int indent)
 {
     switch (qobject_type(obj)) {
@@ -261,15 +233,29 @@ static void to_json(const QObject *obj, QString *str, int pretty, int indent)
         break;
     }
     case QTYPE_QDICT: {
-        ToJsonIterState s;
         QDict *val = qobject_to(QDict, obj);
+        const char *comma = pretty ? "," : ", ";
+        const char *sep = "";
+        const QDictEntry *entry;
+        QString *qkey;
 
-        s.count = 0;
-        s.str = str;
-        s.indent = indent + 1;
-        s.pretty = pretty;
         qstring_append(str, "{");
-        qdict_iter(val, to_json_dict_iter, &s);
+
+        for (entry = qdict_first(val);
+             entry;
+             entry = qdict_next(val, entry)) {
+            qstring_append(str, sep);
+            json_pretty_newline(str, pretty, indent + 1);
+
+            qkey = qstring_from_str(qdict_entry_key(entry));
+            to_json(QOBJECT(qkey), str, pretty, indent + 1);
+            qobject_unref(qkey);
+
+            qstring_append(str, ": ");
+            to_json(qdict_entry_value(entry), str, pretty, indent + 1);
+            sep = comma;
+        }
+
         json_pretty_newline(str, pretty, indent);
         qstring_append(str, "}");
         break;
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 97172b5eaa..3fabfd157d 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -1015,6 +1015,7 @@ QemuOpts *qemu_opts_from_qdict(QemuOptsList *list, const QDict *qdict,
     OptsFromQDictState state;
     Error *local_err = NULL;
     QemuOpts *opts;
+    const QDictEntry *entry;
 
     opts = qemu_opts_create(list, qdict_get_try_str(qdict, "id"), 1,
                             &local_err);
@@ -1027,7 +1028,14 @@ QemuOpts *qemu_opts_from_qdict(QemuOptsList *list, const QDict *qdict,
 
     state.errp = &local_err;
     state.opts = opts;
-    qdict_iter(qdict, qemu_opts_from_qdict_1, &state);
+
+    for (entry = qdict_first(qdict);
+         entry;
+         entry = qdict_next(qdict, entry)) {
+        qemu_opts_from_qdict_1(qdict_entry_key(entry),
+                               qdict_entry_value(entry),
+                               &state);
+    }
     if (local_err) {
         error_propagate(errp, local_err);
         qemu_opts_del(opts);
-- 
2.21.1

[PATCH for-5.1 5/5] qemu-option: Clean up after the previous commit

[Markus Armbruster]

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 util/qemu-option.c | 43 +++++++++++++++----------------------------
 1 file changed, 15 insertions(+), 28 deletions(-)

diff --git a/util/qemu-option.c b/util/qemu-option.c
index 3fabfd157d..123c52bf07 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -967,18 +967,16 @@ void qemu_opts_set_defaults(QemuOptsList *list, const char *params,
     assert(opts);
 }
 
-typedef struct OptsFromQDictState {
-    QemuOpts *opts;
-    Error **errp;
-} OptsFromQDictState;
-
-static void qemu_opts_from_qdict_1(const char *key, QObject *obj, void *opaque)
+static void qemu_opts_from_qdict_entry(QemuOpts *opts,
+                                       const QDictEntry *entry,
+                                       Error **errp)
 {
-    OptsFromQDictState *state = opaque;
+    const char *key = qdict_entry_key(entry);
+    QObject *obj = qdict_entry_value(entry);
     char buf[32], *tmp = NULL;
     const char *value;
 
-    if (!strcmp(key, "id") || *state->errp) {
+    if (!strcmp(key, "id")) {
         return;
     }
 
@@ -999,7 +997,7 @@ static void qemu_opts_from_qdict_1(const char *key, QObject *obj, void *opaque)
         return;
     }
 
-    qemu_opt_set(state->opts, key, value, state->errp);
+    qemu_opt_set(opts, key, value, errp);
     g_free(tmp);
 }
 
@@ -1012,7 +1010,6 @@ static void qemu_opts_from_qdict_1(const char *key, QObject *obj, void *opaque)
 QemuOpts *qemu_opts_from_qdict(QemuOptsList *list, const QDict *qdict,
                                Error **errp)
 {
-    OptsFromQDictState state;
     Error *local_err = NULL;
     QemuOpts *opts;
     const QDictEntry *entry;
@@ -1026,20 +1023,15 @@ QemuOpts *qemu_opts_from_qdict(QemuOptsList *list, const QDict *qdict,
 
     assert(opts != NULL);
 
-    state.errp = &local_err;
-    state.opts = opts;
-
     for (entry = qdict_first(qdict);
          entry;
          entry = qdict_next(qdict, entry)) {
-        qemu_opts_from_qdict_1(qdict_entry_key(entry),
-                               qdict_entry_value(entry),
-                               &state);
-    }
-    if (local_err) {
-        error_propagate(errp, local_err);
-        qemu_opts_del(opts);
-        return NULL;
+        qemu_opts_from_qdict_entry(opts, entry, &local_err);
+        if (local_err) {
+            error_propagate(errp, local_err);
+            qemu_opts_del(opts);
+            return NULL;
+        }
     }
 
     return opts;
@@ -1058,21 +1050,16 @@ void qemu_opts_absorb_qdict(QemuOpts *opts, QDict *qdict, Error **errp)
 
     while (entry != NULL) {
         Error *local_err = NULL;
-        OptsFromQDictState state = {
-            .errp = &local_err,
-            .opts = opts,
-        };
 
         next = qdict_next(qdict, entry);
 
         if (find_desc_by_name(opts->list->desc, entry->key)) {
-            qemu_opts_from_qdict_1(entry->key, entry->value, &state);
+            qemu_opts_from_qdict_entry(opts, entry, &local_err);
             if (local_err) {
                 error_propagate(errp, local_err);
                 return;
-            } else {
-                qdict_del(qdict, entry->key);
             }
+            qdict_del(qdict, entry->key);
         }
 
         entry = next;
-- 
2.21.1

Re: [PATCH for-5.1 1/5] qobject: Clean up QLIST_FOREACH_ENTRY()

[Eric Blake]

On 4/15/20 3:30 AM, Markus Armbruster wrote:
> QLIST_FOREACH_ENTRY() traverses a tail queue manually.  Use
> QTAILQ_FIRST() and QTAILQ_NEXT() instead.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   include/qapi/qmp/qlist.h | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

> diff --git a/include/qapi/qmp/qlist.h b/include/qapi/qmp/qlist.h
> index 8d2c32ca28..07ecae81e4 100644
> --- a/include/qapi/qmp/qlist.h
> +++ b/include/qapi/qmp/qlist.h
> @@ -34,10 +34,10 @@ void qlist_append_int(QList *qlist, int64_t value);
>   void qlist_append_null(QList *qlist);
>   void qlist_append_str(QList *qlist, const char *value);
>   
> -#define QLIST_FOREACH_ENTRY(qlist, var)             \
> -        for ((var) = ((qlist)->head.tqh_first);     \
> -            (var);                                  \
> -            (var) = ((var)->next.tqe_next))
> +#define QLIST_FOREACH_ENTRY(qlist, var)                 \
> +        for ((var) = QTAILQ_FIRST(&(qlist)->head);      \
> +             (var);                                     \
> +             (var) = QTAILQ_NEXT((var), next))
>   
>   static inline QObject *qlist_entry_obj(const QListEntry *entry)
>   {
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PATCH for-5.1 2/5] qobject: Factor out helper json_pretty_newline()

[Eric Blake]

On 4/15/20 3:30 AM, Markus Armbruster wrote:
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   qobject/qjson.c | 40 ++++++++++++++++------------------------
>   1 file changed, 16 insertions(+), 24 deletions(-)
> 
> diff --git a/qobject/qjson.c b/qobject/qjson.c
> index db36101f3b..f3c62711b9 100644
> --- a/qobject/qjson.c
> +++ b/qobject/qjson.c
> @@ -159,21 +159,28 @@ typedef struct ToJsonIterState
>   
>   static void to_json(const QObject *obj, QString *str, int pretty, int indent);
>   
> +static void json_pretty_newline(QString *str, bool pretty, int indent)
> +{
> +    int i;
> +
> +    if (pretty) {
> +        qstring_append(str, "\n");
> +        for (i = 0 ; i < indent ; i++) {

Why are you keeping the spaces before ; ?  Yes, I know they were 
copied-and-pasted from the old code, but as long as you are refactoring, 
fixing the style is worthwhile.

Otherwise,
Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PATCH for-5.1 3/5] qobject: Eliminate qlist_iter(), use QLIST_FOREACH_ENTRY() instead

[Eric Blake]

On 4/15/20 3:30 AM, Markus Armbruster wrote:
> qlist_iter() has just three uses outside tests/.  Replace by
> QLIST_FOREACH_ENTRY() for more concise code and less type punning.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   include/qapi/qmp/qlist.h |  2 --
>   qobject/qjson.c          | 31 ++++++++++------------------
>   qobject/qlist.c          | 44 +++++++++++-----------------------------
>   tests/check-qlist.c      | 37 +++++++++++++--------------------
>   4 files changed, 37 insertions(+), 77 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PATCH for-5.1 4/5] qobject: Eliminate qdict_iter(), use qdict_first(), qdict_next()

[Eric Blake]

On 4/15/20 3:30 AM, Markus Armbruster wrote:
> qdict_iter() has just three uses and no test coverage.  Replace by
> qdict_first(), qdict_next() for more concise code and less type
> punning.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   include/qapi/qmp/qdict.h     |  3 --
>   qapi/qobject-input-visitor.c | 21 +++++++-------
>   qobject/qdict.c              | 19 -------------
>   qobject/qjson.c              | 54 +++++++++++++-----------------------
>   util/qemu-option.c           | 10 ++++++-
>   5 files changed, 40 insertions(+), 67 deletions(-)
> 

>   static const QListEntry *qobject_input_push(QObjectInputVisitor *qiv,
>                                               const char *name,
>                                               QObject *obj, void *qapi)
>   {
>       GHashTable *h;
>       StackObject *tos = g_new0(StackObject, 1);
> +    QDict *qdict = qobject_to(QDict, obj);
> +    QList *qlist = qobject_to(QList, obj);
> +    const QDictEntry *entry;
>   
>       assert(obj);
>       tos->name = name;
>       tos->obj = obj;
>       tos->qapi = qapi;
>   
> -    if (qobject_type(obj) == QTYPE_QDICT) {
> +    if (qdict) {
>           h = g_hash_table_new(g_str_hash, g_str_equal);
> -        qdict_iter(qobject_to(QDict, obj), qdict_add_key, h);
> +        for (entry = qdict_first(qdict);
> +             entry;
> +             entry = qdict_next(qdict, entry)) {
> +            g_hash_table_insert(h, (void *)qdict_entry_key(entry), NULL);

Is the cast to void* necessary?

Otherwise,
Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PATCH for-5.1 5/5] qemu-option: Clean up after the previous commit

[Eric Blake]

On 4/15/20 3:30 AM, Markus Armbruster wrote:
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   util/qemu-option.c | 43 +++++++++++++++----------------------------
>   1 file changed, 15 insertions(+), 28 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PATCH for-5.1 2/5] qobject: Factor out helper json_pretty_newline()

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> On 4/15/20 3:30 AM, Markus Armbruster wrote:
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>   qobject/qjson.c | 40 ++++++++++++++++------------------------
>>   1 file changed, 16 insertions(+), 24 deletions(-)
>>
>> diff --git a/qobject/qjson.c b/qobject/qjson.c
>> index db36101f3b..f3c62711b9 100644
>> --- a/qobject/qjson.c
>> +++ b/qobject/qjson.c
>> @@ -159,21 +159,28 @@ typedef struct ToJsonIterState
>>     static void to_json(const QObject *obj, QString *str, int
>> pretty, int indent);
>>   +static void json_pretty_newline(QString *str, bool pretty, int
>> indent)
>> +{
>> +    int i;
>> +
>> +    if (pretty) {
>> +        qstring_append(str, "\n");
>> +        for (i = 0 ; i < indent ; i++) {
>
> Why are you keeping the spaces before ; ?  Yes, I know they were
> copied-and-pasted from the old code, but as long as you are
> refactoring, fixing the style is worthwhile.

Makes sense.

> Otherwise,
> Reviewed-by: Eric Blake <eblake@redhat.com>

Thanks!

Re: [PATCH for-5.1 4/5] qobject: Eliminate qdict_iter(), use qdict_first(), qdict_next()

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> On 4/15/20 3:30 AM, Markus Armbruster wrote:
>> qdict_iter() has just three uses and no test coverage.  Replace by
>> qdict_first(), qdict_next() for more concise code and less type
>> punning.
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>   include/qapi/qmp/qdict.h     |  3 --
>>   qapi/qobject-input-visitor.c | 21 +++++++-------
>>   qobject/qdict.c              | 19 -------------
>>   qobject/qjson.c              | 54 +++++++++++++-----------------------
>>   util/qemu-option.c           | 10 ++++++-
>>   5 files changed, 40 insertions(+), 67 deletions(-)
>>
>
>>   static const QListEntry *qobject_input_push(QObjectInputVisitor *qiv,
>>                                               const char *name,
>>                                               QObject *obj, void *qapi)
>>   {
>>       GHashTable *h;
>>       StackObject *tos = g_new0(StackObject, 1);
>> +    QDict *qdict = qobject_to(QDict, obj);
>> +    QList *qlist = qobject_to(QList, obj);
>> +    const QDictEntry *entry;
>>         assert(obj);
>>       tos->name = name;
>>       tos->obj = obj;
>>       tos->qapi = qapi;
>>   -    if (qobject_type(obj) == QTYPE_QDICT) {
>> +    if (qdict) {
>>           h = g_hash_table_new(g_str_hash, g_str_equal);
>> -        qdict_iter(qobject_to(QDict, obj), qdict_add_key, h);
>> +        for (entry = qdict_first(qdict);
>> +             entry;
>> +             entry = qdict_next(qdict, entry)) {
>> +            g_hash_table_insert(h, (void *)qdict_entry_key(entry), NULL);
>
> Is the cast to void* necessary?

It casts away const.

> Otherwise,
> Reviewed-by: Eric Blake <eblake@redhat.com>

Thanks!

Re: [PATCH for-5.1 0/5] qobject: Minor spring cleaning

[Markus Armbruster]

Queued.