[v11,01/15] qapi: Simplify excess input reporting in input visitors

Commit Message

Eric Blake Feb. 18, 2016, 6:48 a.m. UTC

When reporting that an unvisited member remains at the end of an
input visit for a struct, we were using g_hash_table_find()
coupled with a callback function that always returns true, to
locate an arbitrary member of the hash table.  But if all we
need is an arbitrary entry, we can get that from a single-use
iterator, without needing a tautological callback function.

Technically, our cast of &(GQueue *) to (void **) is not strict
C (while void * must be able to hold all other pointers, nothing
says a void ** has to be the same width or representation as a
GQueue **).  The kosher way to write it would be the verbose:

    void *tmp;
    GQueue *any;
    if (g_hash_table_iter_next(&iter, NULL, &tmp)) {
        any = tmp;

But our code base (not to mention glib itself) already has other
cases of assuming that ALL pointers have the same width and
representation, where a compiler would have to go out of its way
to mis-compile our borderline behavior.

Suggested-by: Markus Armbruster <[e-mail]>
Signed-off-by: Eric Blake <[e-mail]>
Reviewed-by: Marc-André Lureau <[e-mail]>

---
v11: no change
v10: enhance commit message
v9: no change
v8: rebase to earlier changes
v7: retitle, rebase to earlier context changes
v6: new patch, based on comments on RFC against v5 7/46
---
 qapi/opts-visitor.c      | 12 +++---------
 qapi/qmp-input-visitor.c | 14 +++++---------
 2 files changed, 8 insertions(+), 18 deletions(-)

Patch

diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
index d54f75b..ae5b955 100644
--- a/qapi/opts-visitor.c
+++ b/qapi/opts-visitor.c
@@ -157,17 +157,11 @@  opts_start_struct(Visitor *v, const char *name, void **obj,
 }


-static gboolean
-ghr_true(gpointer ign_key, gpointer ign_value, gpointer ign_user_data)
-{
-    return TRUE;
-}
-
-
 static void
 opts_end_struct(Visitor *v, Error **errp)
 {
     OptsVisitor *ov = to_ov(v);
+    GHashTableIter iter;
     GQueue *any;

     if (--ov->depth > 0) {
@@ -175,8 +169,8 @@  opts_end_struct(Visitor *v, Error **errp)
     }

     /* we should have processed all (distinct) QemuOpt instances */
-    any = g_hash_table_find(ov->unprocessed_opts, &ghr_true, NULL);
-    if (any) {
+    g_hash_table_iter_init(&iter, ov->unprocessed_opts);
+    if (g_hash_table_iter_next(&iter, NULL, (void **)&any)) {
         const QemuOpt *first;

         first = g_queue_peek_head(any);
diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
index 362a1a3..2f48b95 100644
--- a/qapi/qmp-input-visitor.c
+++ b/qapi/qmp-input-visitor.c
@@ -90,12 +90,6 @@  static void qmp_input_push(QmpInputVisitor *qiv, QObject *obj, Error **errp)
     qiv->nb_stack++;
 }

-/** Only for qmp_input_pop. */
-static gboolean always_true(gpointer key, gpointer val, gpointer user_pkey)
-{
-    *(const char **)user_pkey = (const char *)key;
-    return TRUE;
-}

 static void qmp_input_pop(QmpInputVisitor *qiv, Error **errp)
 {
@@ -104,9 +98,11 @@  static void qmp_input_pop(QmpInputVisitor *qiv, Error **errp)
     if (qiv->strict) {
         GHashTable * const top_ht = qiv->stack[qiv->nb_stack - 1].h;
         if (top_ht) {
-            if (g_hash_table_size(top_ht)) {
-                const char *key;
-                g_hash_table_find(top_ht, always_true, &key);
+            GHashTableIter iter;
+            const char *key;
+
+            g_hash_table_iter_init(&iter, top_ht);
+            if (g_hash_table_iter_next(&iter, (void **)&key, NULL)) {
                 error_setg(errp, QERR_QMP_EXTRA_MEMBER, key);
             }
             g_hash_table_unref(top_ht);