From 99fb0c53c038105bae68b02a3d9f1cbf7951ba10 Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Fri, 3 Mar 2017 13:32:29 +0100
Subject: qmp: Eliminate silly QERR_QMP_* macros

The QERR_ macros are leftovers from the days of "rich" error objects.

QERR_QMP_BAD_INPUT_OBJECT, QERR_QMP_BAD_INPUT_OBJECT_MEMBER,
QERR_QMP_EXTRA_MEMBER are used in just one place now, except for one
use that has crept into qobject-input-visitor.c.

Drop these macros, to make the (bad) error messages more visible.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1488544368-30622-10-git-send-email-armbru@redhat.com>
---
 qapi/qobject-input-visitor.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

(limited to 'qapi/qobject-input-visitor.c')

diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
index 0063327b3b..ed6c24ca44 100644
--- a/qapi/qobject-input-visitor.c
+++ b/qapi/qobject-input-visitor.c
@@ -140,7 +140,8 @@ static void qobject_input_check_struct(Visitor *v, Error **errp)
 
             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);
+                error_setg(errp, "QMP input object member '%s' is unexpected",
+                           key);
             }
         }
     }
-- 
cgit 1.4.1


From 910f738b851a263396fc85b2052e47f884ffead3 Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Fri, 3 Mar 2017 13:32:31 +0100
Subject: qapi: Improve a QObject input visitor error message

The QObject input visitor has three error message formats:

* Parameter '%s' is missing
* "Invalid parameter type for '%s', expected: %s"
* "QMP input object member '%s' is unexpected"

The '%s' are member names (or "null", but I'll fix that later).

The last error message calls the thing "QMP input object member"
instead of "parameter".  Misleading when the visitor is used on
QObjects that don't come from QMP.  Change it to "Parameter '%s' is
unexpected".

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1488544368-30622-12-git-send-email-armbru@redhat.com>
---
 qapi/qobject-input-visitor.c | 3 +--
 tests/test-qga.c             | 2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

(limited to 'qapi/qobject-input-visitor.c')

diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
index ed6c24ca44..f3b6713f94 100644
--- a/qapi/qobject-input-visitor.c
+++ b/qapi/qobject-input-visitor.c
@@ -140,8 +140,7 @@ static void qobject_input_check_struct(Visitor *v, Error **errp)
 
             g_hash_table_iter_init(&iter, top_ht);
             if (g_hash_table_iter_next(&iter, (void **)&key, NULL)) {
-                error_setg(errp, "QMP input object member '%s' is unexpected",
-                           key);
+                error_setg(errp, "Parameter '%s' is unexpected", key);
             }
         }
     }
diff --git a/tests/test-qga.c b/tests/test-qga.c
index 868b02a40f..ae97b575c1 100644
--- a/tests/test-qga.c
+++ b/tests/test-qga.c
@@ -213,7 +213,7 @@ static void test_qga_invalid_args(gconstpointer fix)
     desc = qdict_get_try_str(error, "desc");
 
     g_assert_cmpstr(class, ==, "GenericError");
-    g_assert_cmpstr(desc, ==, "QMP input object member 'foo' is unexpected");
+    g_assert_cmpstr(desc, ==, "Parameter 'foo' is unexpected");
 
     QDECREF(ret);
 }
-- 
cgit 1.4.1


From b8874fbfd329b5084463bcacd1418d493a93c383 Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Fri, 3 Mar 2017 13:32:32 +0100
Subject: qapi: Clean up after commit 3d344c2

Drop unused QIV_STACK_SIZE and unused qobject_input_start_struct()
parameter errp.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1488544368-30622-13-git-send-email-armbru@redhat.com>
---
 qapi/qobject-input-visitor.c | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

(limited to 'qapi/qobject-input-visitor.c')

diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
index f3b6713f94..2c2f88338f 100644
--- a/qapi/qobject-input-visitor.c
+++ b/qapi/qobject-input-visitor.c
@@ -21,8 +21,6 @@
 #include "qapi/qmp/types.h"
 #include "qapi/qmp/qerror.h"
 
-#define QIV_STACK_SIZE 1024
-
 typedef struct StackObject
 {
     QObject *obj; /* Object being visited */
@@ -103,8 +101,7 @@ static void qdict_add_key(const char *key, QObject *obj, void *opaque)
 }
 
 static const QListEntry *qobject_input_push(QObjectInputVisitor *qiv,
-                                            QObject *obj, void *qapi,
-                                            Error **errp)
+                                            QObject *obj, void *qapi)
 {
     GHashTable *h;
     StackObject *tos = g_new0(StackObject, 1);
@@ -170,7 +167,6 @@ static void qobject_input_start_struct(Visitor *v, const char *name, void **obj,
 {
     QObjectInputVisitor *qiv = to_qiv(v);
     QObject *qobj = qobject_input_get_object(qiv, name, true, errp);
-    Error *err = NULL;
 
     if (obj) {
         *obj = NULL;
@@ -184,11 +180,7 @@ static void qobject_input_start_struct(Visitor *v, const char *name, void **obj,
         return;
     }
 
-    qobject_input_push(qiv, qobj, obj, &err);
-    if (err) {
-        error_propagate(errp, err);
-        return;
-    }
+    qobject_input_push(qiv, qobj, obj);
 
     if (obj) {
         *obj = g_malloc0(size);
@@ -216,7 +208,7 @@ static void qobject_input_start_list(Visitor *v, const char *name,
         return;
     }
 
-    entry = qobject_input_push(qiv, qobj, list, errp);
+    entry = qobject_input_push(qiv, qobj, list);
     if (list) {
         if (entry) {
             *list = g_malloc0(size);
-- 
cgit 1.4.1


From 58561c27669ddf1c6d39ff8ce25837c6f2d9d92c Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Fri, 3 Mar 2017 13:32:33 +0100
Subject: qapi: Make QObject input visitor set *list reliably
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

qobject_input_start_struct() sets *list, except when it fails because
qobject_input_get_object() fails, i.e. the input object doesn't exist.

All the other input visitor start_struct(), start_list(),
start_alternate() always set *obj / *list.

Change qobject_input_start_struct() to match.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1488544368-30622-14-git-send-email-armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 qapi/qobject-input-visitor.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

(limited to 'qapi/qobject-input-visitor.c')

diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
index 2c2f88338f..d58696c7df 100644
--- a/qapi/qobject-input-visitor.c
+++ b/qapi/qobject-input-visitor.c
@@ -196,25 +196,21 @@ static void qobject_input_start_list(Visitor *v, const char *name,
     QObject *qobj = qobject_input_get_object(qiv, name, true, errp);
     const QListEntry *entry;
 
+    if (list) {
+        *list = NULL;
+    }
     if (!qobj) {
         return;
     }
     if (qobject_type(qobj) != QTYPE_QLIST) {
-        if (list) {
-            *list = NULL;
-        }
         error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
                    "list");
         return;
     }
 
     entry = qobject_input_push(qiv, qobj, list);
-    if (list) {
-        if (entry) {
-            *list = g_malloc0(size);
-        } else {
-            *list = NULL;
-        }
+    if (entry && list) {
+        *list = g_malloc0(size);
     }
 }
 
-- 
cgit 1.4.1


From a9fc37f6bc3f2ab90585cb16493da9f6dcfbfbcf Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Fri, 3 Mar 2017 13:32:34 +0100
Subject: qapi: Improve qobject input visitor error reporting

Error messages refer to nodes of the QObject being visited by name.
Trouble is the names are sometimes less than helpful:

* The name of the root QObject is whatever @name argument got passed
  to the visitor, except NULL gets mapped to "null".  We commonly pass
  NULL.  Not good.

  Avoiding errors "at the root" mitigates.  For instance,
  visit_start_struct() can only fail when the visited object is not a
  dictionary, and we commonly ensure it is beforehand.

* The name of a QDict's member is the member key.  Good enough only
  when this happens to be unique.

* The name of a QList's member is "null".  Not good.

Improve error messages by referring to nodes by path instead, as
follows:

* The path of the root QObject is whatever @name argument got passed
  to the visitor, except NULL gets mapped to "<anonymous>".

* The path of a root QDict's member is the member key.

* The path of a root QList's member is "[%u]", where %u is the list
  index, starting at zero.

* The path of a non-root QDict's member is the path of the QDict
  concatenated with "." and the member key.

* The path of a non-root QList's member is the path of the QList
  concatenated with "[%u]", where %u is the list index.

For example, the incorrect QMP command

    { "execute": "blockdev-add", "arguments": { "node-name": "foo", "driver": "raw", "file": {"driver": "file" } } }

now fails with

    {"error": {"class": "GenericError", "desc": "Parameter 'file.filename' is missing"}}

instead of

    {"error": {"class": "GenericError", "desc": "Parameter 'filename' is missing"}}

and

    { "execute": "input-send-event", "arguments": { "device": "bar", "events": [ [] ] } }

now fails with

    {"error": {"class": "GenericError", "desc": "Invalid parameter type for 'events[0]', expected: object"}}

instead of

    {"error": {"class": "GenericError", "desc": "Invalid parameter type for 'null', expected: QDict"}}

Aside: calling the thing "parameter" is suboptimal for QMP, because
the root object is "arguments" there.

The qobject output visitor doesn't have this problem because it should
not fail.  Same for dealloc and clone visitors.

The string visitors don't have this problem because they visit just
one value, whose name needs to be passed to the visitor as @name.  The
string output visitor shouldn't fail anyway.

The options visitor uses QemuOpts names.  Their name space is flat, so
the use of QDict member keys as names is fine.  NULL names used with
roots and lists could conceivably result in bad error messages.  Left
for another day.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1488544368-30622-15-git-send-email-armbru@redhat.com>
---
 include/qapi/visitor.h       |   6 ---
 qapi/qobject-input-visitor.c | 121 +++++++++++++++++++++++++++++++------------
 2 files changed, 87 insertions(+), 40 deletions(-)

(limited to 'qapi/qobject-input-visitor.c')

diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index 9bb6cba237..7c91a50a4c 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -66,12 +66,6 @@
  * object, @name is the key associated with the value; and when
  * visiting a member of a list, @name is NULL.
  *
- * FIXME: Clients must pass NULL for @name when visiting a member of a
- * list, but this leads to poor error messages; it might be nicer to
- * require a non-NULL name such as "key.0" for '{ "key": [ "value" ]
- * }' if an error is encountered on "value" (or to have the visitor
- * core auto-generate the nicer name).
- *
  * The visit_type_FOO() functions expect a non-null @obj argument;
  * they allocate *@obj during input visits, leave it unchanged on
  * output visits, and recursively free any resources during a dealloc
diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
index d58696c7df..8015a986b0 100644
--- a/qapi/qobject-input-visitor.c
+++ b/qapi/qobject-input-visitor.c
@@ -21,19 +21,19 @@
 #include "qapi/qmp/types.h"
 #include "qapi/qmp/qerror.h"
 
-typedef struct StackObject
-{
-    QObject *obj; /* Object being visited */
+typedef struct StackObject {
+    const char *name;            /* Name of @obj in its parent, if any */
+    QObject *obj;                /* QDict or QList being visited */
     void *qapi; /* sanity check that caller uses same pointer */
 
-    GHashTable *h;           /* If obj is dict: unvisited keys */
-    const QListEntry *entry; /* If obj is list: unvisited tail */
+    GHashTable *h;              /* If @obj is QDict: unvisited keys */
+    const QListEntry *entry;    /* If @obj is QList: unvisited tail */
+    unsigned index;             /* If @obj is QList: list index of @entry */
 
-    QSLIST_ENTRY(StackObject) node;
+    QSLIST_ENTRY(StackObject) node; /* parent */
 } StackObject;
 
-struct QObjectInputVisitor
-{
+struct QObjectInputVisitor {
     Visitor visitor;
 
     /* Root of visit at visitor creation. */
@@ -45,6 +45,8 @@ struct QObjectInputVisitor
 
     /* True to reject parse in visit_end_struct() if unvisited keys remain. */
     bool strict;
+
+    GString *errname;           /* Accumulator for full_name() */
 };
 
 static QObjectInputVisitor *to_qiv(Visitor *v)
@@ -52,9 +54,42 @@ static QObjectInputVisitor *to_qiv(Visitor *v)
     return container_of(v, QObjectInputVisitor, visitor);
 }
 
-static QObject *qobject_input_get_object(QObjectInputVisitor *qiv,
-                                         const char *name,
-                                         bool consume, Error **errp)
+static const char *full_name(QObjectInputVisitor *qiv, const char *name)
+{
+    StackObject *so;
+    char buf[32];
+
+    if (qiv->errname) {
+        g_string_truncate(qiv->errname, 0);
+    } else {
+        qiv->errname = g_string_new("");
+    }
+
+    QSLIST_FOREACH(so , &qiv->stack, node) {
+        if (qobject_type(so->obj) == QTYPE_QDICT) {
+            g_string_prepend(qiv->errname, name);
+            g_string_prepend_c(qiv->errname, '.');
+        } else {
+            snprintf(buf, sizeof(buf), "[%u]", so->index);
+            g_string_prepend(qiv->errname, buf);
+        }
+        name = so->name;
+    }
+
+    if (name) {
+        g_string_prepend(qiv->errname, name);
+    } else if (qiv->errname->str[0] == '.') {
+        g_string_erase(qiv->errname, 0, 1);
+    } else {
+        return "<anonymous>";
+    }
+
+    return qiv->errname->str;
+}
+
+static QObject *qobject_input_try_get_object(QObjectInputVisitor *qiv,
+                                             const char *name,
+                                             bool consume)
 {
     StackObject *tos;
     QObject *qobj;
@@ -78,9 +113,6 @@ static QObject *qobject_input_get_object(QObjectInputVisitor *qiv,
             bool removed = g_hash_table_remove(tos->h, name);
             assert(removed);
         }
-        if (!ret) {
-            error_setg(errp, QERR_MISSING_PARAMETER, name);
-        }
     } else {
         assert(qobject_type(qobj) == QTYPE_QLIST);
         assert(!name);
@@ -88,12 +120,25 @@ static QObject *qobject_input_get_object(QObjectInputVisitor *qiv,
         assert(ret);
         if (consume) {
             tos->entry = qlist_next(tos->entry);
+            tos->index++;
         }
     }
 
     return ret;
 }
 
+static QObject *qobject_input_get_object(QObjectInputVisitor *qiv,
+                                         const char *name,
+                                         bool consume, Error **errp)
+{
+    QObject *obj = qobject_input_try_get_object(qiv, name, consume);
+
+    if (!obj) {
+        error_setg(errp, QERR_MISSING_PARAMETER, full_name(qiv, name));
+    }
+    return obj;
+}
+
 static void qdict_add_key(const char *key, QObject *obj, void *opaque)
 {
     GHashTable *h = opaque;
@@ -101,12 +146,14 @@ static void qdict_add_key(const char *key, QObject *obj, void *opaque)
 }
 
 static const QListEntry *qobject_input_push(QObjectInputVisitor *qiv,
+                                            const char *name,
                                             QObject *obj, void *qapi)
 {
     GHashTable *h;
     StackObject *tos = g_new0(StackObject, 1);
 
     assert(obj);
+    tos->name = name;
     tos->obj = obj;
     tos->qapi = qapi;
 
@@ -116,6 +163,7 @@ static const QListEntry *qobject_input_push(QObjectInputVisitor *qiv,
         tos->h = h;
     } else if (qobject_type(obj) == QTYPE_QLIST) {
         tos->entry = qlist_first(qobject_to_qlist(obj));
+        tos->index = -1;
     }
 
     QSLIST_INSERT_HEAD(&qiv->stack, tos, node);
@@ -137,7 +185,8 @@ static void qobject_input_check_struct(Visitor *v, Error **errp)
 
             g_hash_table_iter_init(&iter, top_ht);
             if (g_hash_table_iter_next(&iter, (void **)&key, NULL)) {
-                error_setg(errp, "Parameter '%s' is unexpected", key);
+                error_setg(errp, "Parameter '%s' is unexpected",
+                           full_name(qiv, key));
             }
         }
     }
@@ -175,12 +224,12 @@ static void qobject_input_start_struct(Visitor *v, const char *name, void **obj,
         return;
     }
     if (qobject_type(qobj) != QTYPE_QDICT) {
-        error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
-                   "QDict");
+        error_setg(errp, QERR_INVALID_PARAMETER_TYPE,
+                   full_name(qiv, name), "object");
         return;
     }
 
-    qobject_input_push(qiv, qobj, obj);
+    qobject_input_push(qiv, name, qobj, obj);
 
     if (obj) {
         *obj = g_malloc0(size);
@@ -203,12 +252,12 @@ static void qobject_input_start_list(Visitor *v, const char *name,
         return;
     }
     if (qobject_type(qobj) != QTYPE_QLIST) {
-        error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
-                   "list");
+        error_setg(errp, QERR_INVALID_PARAMETER_TYPE,
+                   full_name(qiv, name), "array");
         return;
     }
 
-    entry = qobject_input_push(qiv, qobj, list);
+    entry = qobject_input_push(qiv, name, qobj, list);
     if (entry && list) {
         *list = g_malloc0(size);
     }
@@ -258,8 +307,8 @@ static void qobject_input_type_int64(Visitor *v, const char *name, int64_t *obj,
     }
     qint = qobject_to_qint(qobj);
     if (!qint) {
-        error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
-                   "integer");
+        error_setg(errp, QERR_INVALID_PARAMETER_TYPE,
+                   full_name(qiv, name), "integer");
         return;
     }
 
@@ -279,8 +328,8 @@ static void qobject_input_type_uint64(Visitor *v, const char *name,
     }
     qint = qobject_to_qint(qobj);
     if (!qint) {
-        error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
-                   "integer");
+        error_setg(errp, QERR_INVALID_PARAMETER_TYPE,
+                   full_name(qiv, name), "integer");
         return;
     }
 
@@ -299,8 +348,8 @@ static void qobject_input_type_bool(Visitor *v, const char *name, bool *obj,
     }
     qbool = qobject_to_qbool(qobj);
     if (!qbool) {
-        error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
-                   "boolean");
+        error_setg(errp, QERR_INVALID_PARAMETER_TYPE,
+                   full_name(qiv, name), "boolean");
         return;
     }
 
@@ -320,8 +369,8 @@ static void qobject_input_type_str(Visitor *v, const char *name, char **obj,
     }
     qstr = qobject_to_qstring(qobj);
     if (!qstr) {
-        error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
-                   "string");
+        error_setg(errp, QERR_INVALID_PARAMETER_TYPE,
+                   full_name(qiv, name), "string");
         return;
     }
 
@@ -351,8 +400,8 @@ static void qobject_input_type_number(Visitor *v, const char *name, double *obj,
         return;
     }
 
-    error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
-               "number");
+    error_setg(errp, QERR_INVALID_PARAMETER_TYPE,
+               full_name(qiv, name), "number");
 }
 
 static void qobject_input_type_any(Visitor *v, const char *name, QObject **obj,
@@ -380,15 +429,15 @@ static void qobject_input_type_null(Visitor *v, const char *name, Error **errp)
     }
 
     if (qobject_type(qobj) != QTYPE_QNULL) {
-        error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
-                   "null");
+        error_setg(errp, QERR_INVALID_PARAMETER_TYPE,
+                   full_name(qiv, name), "null");
     }
 }
 
 static void qobject_input_optional(Visitor *v, const char *name, bool *present)
 {
     QObjectInputVisitor *qiv = to_qiv(v);
-    QObject *qobj = qobject_input_get_object(qiv, name, false, NULL);
+    QObject *qobj = qobject_input_try_get_object(qiv, name, false);
 
     if (!qobj) {
         *present = false;
@@ -401,6 +450,7 @@ static void qobject_input_optional(Visitor *v, const char *name, bool *present)
 static void qobject_input_free(Visitor *v)
 {
     QObjectInputVisitor *qiv = to_qiv(v);
+
     while (!QSLIST_EMPTY(&qiv->stack)) {
         StackObject *tos = QSLIST_FIRST(&qiv->stack);
 
@@ -409,6 +459,9 @@ static void qobject_input_free(Visitor *v)
     }
 
     qobject_decref(qiv->root);
+    if (qiv->errname) {
+        g_string_free(qiv->errname, TRUE);
+    }
     g_free(qiv);
 }
 
-- 
cgit 1.4.1


From 048abb7b20c9f822ad9d4b730bade73b3311a47a Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Fri, 3 Mar 2017 13:32:39 +0100
Subject: qapi: Drop unused non-strict qobject input visitor

The split between tests/test-qobject-input-visitor.c and
tests/test-qobject-input-strict.c now makes less sense than ever.  The
next commit will take care of that.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1488544368-30622-20-git-send-email-armbru@redhat.com>
---
 block/nbd.c                          |  2 +-
 block/nfs.c                          |  2 +-
 block/ssh.c                          |  2 +-
 docs/qapi-code-gen.txt               |  2 +-
 include/qapi/qobject-input-visitor.h |  5 +----
 qapi/qobject-input-visitor.c         | 30 +++++++++++-------------------
 qmp.c                                |  2 +-
 qom/qom-qobject.c                    |  2 +-
 scripts/qapi-commands.py             |  2 +-
 target/s390x/cpu_models.c            |  2 +-
 tests/check-qnull.c                  |  2 +-
 tests/qmp-test.c                     |  2 +-
 tests/test-qmp-commands.c            |  2 +-
 tests/test-qobject-input-strict.c    |  2 +-
 tests/test-qobject-input-visitor.c   |  2 +-
 tests/test-visitor-serialization.c   |  2 +-
 16 files changed, 26 insertions(+), 37 deletions(-)

(limited to 'qapi/qobject-input-visitor.c')

diff --git a/block/nbd.c b/block/nbd.c
index a7f9108fe5..f478f80b4a 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -278,7 +278,7 @@ static SocketAddress *nbd_config(BDRVNBDState *s, QDict *options, Error **errp)
         goto done;
     }
 
-    iv = qobject_input_visitor_new(crumpled_addr, true);
+    iv = qobject_input_visitor_new(crumpled_addr);
     visit_type_SocketAddress(iv, NULL, &saddr, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
diff --git a/block/nfs.c b/block/nfs.c
index 890d5d4aff..3f43f6e26a 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -474,7 +474,7 @@ static NFSServer *nfs_config(QDict *options, Error **errp)
         goto out;
     }
 
-    iv = qobject_input_visitor_new(crumpled_addr, true);
+    iv = qobject_input_visitor_new(crumpled_addr);
     visit_type_NFSServer(iv, NULL, &server, &local_error);
     if (local_error) {
         error_propagate(errp, local_error);
diff --git a/block/ssh.c b/block/ssh.c
index 835932e6a4..278e66faa6 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -601,7 +601,7 @@ static InetSocketAddress *ssh_config(QDict *options, Error **errp)
         goto out;
     }
 
-    iv = qobject_input_visitor_new(crumpled_addr, true);
+    iv = qobject_input_visitor_new(crumpled_addr);
     visit_type_InetSocketAddress(iv, NULL, &inet, &local_error);
     if (local_error) {
         error_propagate(errp, local_error);
diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index 7eb7be12ab..6746c1052c 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -1138,7 +1138,7 @@ Example:
         Visitor *v;
         UserDefOneList *arg1 = NULL;
 
-        v = qobject_input_visitor_new(QOBJECT(args), true);
+        v = qobject_input_visitor_new(QOBJECT(args));
         visit_start_struct(v, NULL, NULL, 0, &err);
         if (err) {
             goto out;
diff --git a/include/qapi/qobject-input-visitor.h b/include/qapi/qobject-input-visitor.h
index cde328da9f..21db9c4b4a 100644
--- a/include/qapi/qobject-input-visitor.h
+++ b/include/qapi/qobject-input-visitor.h
@@ -21,10 +21,7 @@ typedef struct QObjectInputVisitor QObjectInputVisitor;
 
 /*
  * Return a new input visitor that converts a QObject to a QAPI object.
- *
- * Set @strict to reject a parse that doesn't consume all keys of a
- * dictionary; otherwise excess input is ignored.
  */
-Visitor *qobject_input_visitor_new(QObject *obj, bool strict);
+Visitor *qobject_input_visitor_new(QObject *obj);
 
 #endif
diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
index 8015a986b0..eafcdf4625 100644
--- a/qapi/qobject-input-visitor.c
+++ b/qapi/qobject-input-visitor.c
@@ -43,9 +43,6 @@ struct QObjectInputVisitor {
      * QDict or QList). */
     QSLIST_HEAD(, StackObject) stack;
 
-    /* True to reject parse in visit_end_struct() if unvisited keys remain. */
-    bool strict;
-
     GString *errname;           /* Accumulator for full_name() */
 };
 
@@ -157,11 +154,12 @@ static const QListEntry *qobject_input_push(QObjectInputVisitor *qiv,
     tos->obj = obj;
     tos->qapi = qapi;
 
-    if (qiv->strict && qobject_type(obj) == QTYPE_QDICT) {
+    if (qobject_type(obj) == QTYPE_QDICT) {
         h = g_hash_table_new(g_str_hash, g_str_equal);
         qdict_iter(qobject_to_qdict(obj), qdict_add_key, h);
         tos->h = h;
-    } else if (qobject_type(obj) == QTYPE_QLIST) {
+    } else {
+        assert(qobject_type(obj) == QTYPE_QLIST);
         tos->entry = qlist_first(qobject_to_qlist(obj));
         tos->index = -1;
     }
@@ -175,20 +173,15 @@ static void qobject_input_check_struct(Visitor *v, Error **errp)
 {
     QObjectInputVisitor *qiv = to_qiv(v);
     StackObject *tos = QSLIST_FIRST(&qiv->stack);
+    GHashTableIter iter;
+    const char *key;
 
     assert(tos && !tos->entry);
-    if (qiv->strict) {
-        GHashTable *const top_ht = tos->h;
-        if (top_ht) {
-            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, "Parameter '%s' is unexpected",
-                           full_name(qiv, key));
-            }
-        }
+
+    g_hash_table_iter_init(&iter, tos->h);
+    if (g_hash_table_iter_next(&iter, (void **)&key, NULL)) {
+        error_setg(errp, "Parameter '%s' is unexpected",
+                   full_name(qiv, key));
     }
 }
 
@@ -465,7 +458,7 @@ static void qobject_input_free(Visitor *v)
     g_free(qiv);
 }
 
-Visitor *qobject_input_visitor_new(QObject *obj, bool strict)
+Visitor *qobject_input_visitor_new(QObject *obj)
 {
     QObjectInputVisitor *v;
 
@@ -489,7 +482,6 @@ Visitor *qobject_input_visitor_new(QObject *obj, bool strict)
     v->visitor.type_null = qobject_input_type_null;
     v->visitor.optional = qobject_input_optional;
     v->visitor.free = qobject_input_free;
-    v->strict = strict;
 
     v->root = obj;
     qobject_incref(obj);
diff --git a/qmp.c b/qmp.c
index dfaabac1a6..fa82b598c6 100644
--- a/qmp.c
+++ b/qmp.c
@@ -675,7 +675,7 @@ void qmp_object_add(const char *type, const char *id,
         pdict = qdict_new();
     }
 
-    v = qobject_input_visitor_new(QOBJECT(pdict), true);
+    v = qobject_input_visitor_new(QOBJECT(pdict));
     obj = user_creatable_add_type(type, id, pdict, v, errp);
     visit_free(v);
     if (obj) {
diff --git a/qom/qom-qobject.c b/qom/qom-qobject.c
index bbdedda74a..4aec20d73c 100644
--- a/qom/qom-qobject.c
+++ b/qom/qom-qobject.c
@@ -23,7 +23,7 @@ void object_property_set_qobject(Object *obj, QObject *value,
 {
     Visitor *v;
 
-    v = qobject_input_visitor_new(value, true);
+    v = qobject_input_visitor_new(value);
     object_property_set(obj, v, name, errp);
     visit_free(v);
 }
diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 2b062ad1fd..0c05449cb6 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -130,7 +130,7 @@ def gen_marshal(name, arg_type, boxed, ret_type):
         push_indent()
 
     ret += mcgen('''
-    v = qobject_input_visitor_new(QOBJECT(args), true);
+    v = qobject_input_visitor_new(QOBJECT(args));
     visit_start_struct(v, NULL, NULL, 0, &err);
     if (err) {
         goto out;
diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index 2a894eec65..4ea3a2de80 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -346,7 +346,7 @@ static void cpu_model_from_info(S390CPUModel *model, const CpuModelInfo *info,
     }
 
     if (qdict) {
-        visitor = qobject_input_visitor_new(info->props, true);
+        visitor = qobject_input_visitor_new(info->props);
         visit_start_struct(visitor, NULL, NULL, 0, errp);
         if (*errp) {
             object_unref(obj);
diff --git a/tests/check-qnull.c b/tests/check-qnull.c
index b50bb8a81a..8dd1c9686f 100644
--- a/tests/check-qnull.c
+++ b/tests/check-qnull.c
@@ -47,7 +47,7 @@ static void qnull_visit_test(void)
 
     g_assert(qnull_.refcnt == 1);
     obj = qnull();
-    v = qobject_input_visitor_new(obj, true);
+    v = qobject_input_visitor_new(obj);
     qobject_decref(obj);
     visit_type_null(v, NULL, &error_abort);
     visit_free(v);
diff --git a/tests/qmp-test.c b/tests/qmp-test.c
index 405e49ec34..5d0260b2be 100644
--- a/tests/qmp-test.c
+++ b/tests/qmp-test.c
@@ -34,7 +34,7 @@ static void test_version(QObject *version)
     VersionInfo *vinfo;
 
     g_assert(version);
-    v = qobject_input_visitor_new(version, true);
+    v = qobject_input_visitor_new(version);
     visit_type_VersionInfo(v, "version", &vinfo, &error_abort);
     qapi_free_VersionInfo(vinfo);
     visit_free(v);
diff --git a/tests/test-qmp-commands.c b/tests/test-qmp-commands.c
index 1cf413bc39..0f81a98be2 100644
--- a/tests/test-qmp-commands.c
+++ b/tests/test-qmp-commands.c
@@ -246,7 +246,7 @@ static void test_dealloc_partial(void)
         ud2_dict = qdict_new();
         qdict_put_obj(ud2_dict, "string0", QOBJECT(qstring_from_str(text)));
 
-        v = qobject_input_visitor_new(QOBJECT(ud2_dict), true);
+        v = qobject_input_visitor_new(QOBJECT(ud2_dict));
         visit_type_UserDefTwo(v, NULL, &ud2, &err);
         visit_free(v);
         QDECREF(ud2_dict);
diff --git a/tests/test-qobject-input-strict.c b/tests/test-qobject-input-strict.c
index 4087ea3dd3..7d26113013 100644
--- a/tests/test-qobject-input-strict.c
+++ b/tests/test-qobject-input-strict.c
@@ -53,7 +53,7 @@ static Visitor *validate_test_init_internal(TestInputVisitorData *data,
     data->obj = qobject_from_jsonv(json_string, ap);
     g_assert(data->obj);
 
-    data->qiv = qobject_input_visitor_new(data->obj, true);
+    data->qiv = qobject_input_visitor_new(data->obj);
     g_assert(data->qiv);
     return data->qiv;
 }
diff --git a/tests/test-qobject-input-visitor.c b/tests/test-qobject-input-visitor.c
index 125e34c101..658fa2c24d 100644
--- a/tests/test-qobject-input-visitor.c
+++ b/tests/test-qobject-input-visitor.c
@@ -49,7 +49,7 @@ static Visitor *visitor_input_test_init_internal(TestInputVisitorData *data,
     data->obj = qobject_from_jsonv(json_string, ap);
     g_assert(data->obj);
 
-    data->qiv = qobject_input_visitor_new(data->obj, true);
+    data->qiv = qobject_input_visitor_new(data->obj);
     g_assert(data->qiv);
     return data->qiv;
 }
diff --git a/tests/test-visitor-serialization.c b/tests/test-visitor-serialization.c
index 66b2b1c564..c7e64f022c 100644
--- a/tests/test-visitor-serialization.c
+++ b/tests/test-visitor-serialization.c
@@ -1040,7 +1040,7 @@ static void qmp_deserialize(void **native_out, void *datap,
     obj = qobject_from_json(qstring_get_str(output_json));
 
     QDECREF(output_json);
-    d->qiv = qobject_input_visitor_new(obj, true);
+    d->qiv = qobject_input_visitor_new(obj);
     qobject_decref(obj_orig);
     qobject_decref(obj);
     visit(d->qiv, native_out, errp);
-- 
cgit 1.4.1


From a4a1c70dc759e5b81627e96564f344ab43ea86eb Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Fri, 3 Mar 2017 13:32:45 +0100
Subject: qapi: Make input visitors detect unvisited list tails

Fix the design flaw demonstrated in the previous commit: new method
check_list() lets input visitors report that unvisited input remains
for a list, exactly like check_struct() lets them report that
unvisited input remains for a struct or union.

Implement the method for the qobject input visitor (straightforward),
and the string input visitor (less so, due to the magic list syntax
there).  The opts visitor's list magic is even more impenetrable, and
all I can do there today is a stub with a FIXME comment.  No worse
than before.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1488544368-30622-26-git-send-email-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 hw/ppc/spapr_drc.c                 |  5 +++++
 include/qapi/visitor-impl.h        |  3 +++
 include/qapi/visitor.h             | 13 +++++++++++++
 qapi/opts-visitor.c                | 11 +++++++++++
 qapi/qapi-visit-core.c             |  8 ++++++++
 qapi/qobject-input-visitor.c       | 37 +++++++++++++++++++++++++++++++------
 qapi/string-input-visitor.c        | 30 ++++++++++++++++++++++++++++++
 qapi/trace-events                  |  1 +
 scripts/qapi-visit.py              |  3 +++
 tests/test-opts-visitor.c          |  2 +-
 tests/test-qobject-input-visitor.c |  9 +++++++--
 tests/test-string-input-visitor.c  |  4 +++-
 12 files changed, 116 insertions(+), 10 deletions(-)

(limited to 'qapi/qobject-input-visitor.c')

diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index 2de6377cca..150f6bf2c7 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -326,7 +326,12 @@ static void prop_get_fdt(Object *obj, Visitor *v, const char *name,
                     return;
                 }
             }
+            visit_check_list(v, &err);
             visit_end_list(v, NULL);
+            if (err) {
+                error_propagate(errp, err);
+                return;
+            }
             break;
         }
         default:
diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
index 962ba1df35..e87709db5c 100644
--- a/include/qapi/visitor-impl.h
+++ b/include/qapi/visitor-impl.h
@@ -61,6 +61,9 @@ struct Visitor
     /* Must be set */
     GenericList *(*next_list)(Visitor *v, GenericList *tail, size_t size);
 
+    /* Optional; intended for input visitors */
+    void (*check_list)(Visitor *v, Error **errp);
+
     /* Must be set */
     void (*end_list)(Visitor *v, void **list);
 
diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index 7c91a50a4c..1a1b62012b 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -369,6 +369,19 @@ void visit_start_list(Visitor *v, const char *name, GenericList **list,
  */
 GenericList *visit_next_list(Visitor *v, GenericList *tail, size_t size);
 
+/*
+ * Prepare for completing a list visit.
+ *
+ * @errp obeys typical error usage, and reports failures such as
+ * unvisited list tail remaining in the input stream.
+ *
+ * Should be called prior to visit_end_list() if all other
+ * intermediate visit steps were successful, to allow the visitor one
+ * last chance to report errors.  May be skipped on a cleanup path,
+ * where there is no need to check for further errors.
+ */
+void visit_check_list(Visitor *v, Error **errp);
+
 /*
  * Complete a list visit started earlier.
  *
diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
index c50dc4bbbf..026d25b767 100644
--- a/qapi/opts-visitor.c
+++ b/qapi/opts-visitor.c
@@ -272,6 +272,16 @@ opts_next_list(Visitor *v, GenericList *tail, size_t size)
 }
 
 
+static void
+opts_check_list(Visitor *v, Error **errp)
+{
+    /*
+     * FIXME should set error when unvisited elements remain.  Mostly
+     * harmless, as the generated visits always visit all elements.
+     */
+}
+
+
 static void
 opts_end_list(Visitor *v, void **obj)
 {
@@ -539,6 +549,7 @@ opts_visitor_new(const QemuOpts *opts)
 
     ov->visitor.start_list = &opts_start_list;
     ov->visitor.next_list  = &opts_next_list;
+    ov->visitor.check_list = &opts_check_list;
     ov->visitor.end_list   = &opts_end_list;
 
     ov->visitor.type_int64  = &opts_type_int64;
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index e6e93f02e6..43a09d147d 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -90,6 +90,14 @@ GenericList *visit_next_list(Visitor *v, GenericList *tail, size_t size)
     return v->next_list(v, tail, size);
 }
 
+void visit_check_list(Visitor *v, Error **errp)
+{
+    trace_visit_check_list(v);
+    if (v->check_list) {
+        v->check_list(v, errp);
+    }
+}
+
 void visit_end_list(Visitor *v, void **obj)
 {
     trace_visit_end_list(v, obj);
diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
index eafcdf4625..34065ba7dd 100644
--- a/qapi/qobject-input-visitor.c
+++ b/qapi/qobject-input-visitor.c
@@ -51,7 +51,8 @@ static QObjectInputVisitor *to_qiv(Visitor *v)
     return container_of(v, QObjectInputVisitor, visitor);
 }
 
-static const char *full_name(QObjectInputVisitor *qiv, const char *name)
+static const char *full_name_nth(QObjectInputVisitor *qiv, const char *name,
+                                 int n)
 {
     StackObject *so;
     char buf[32];
@@ -63,8 +64,10 @@ static const char *full_name(QObjectInputVisitor *qiv, const char *name)
     }
 
     QSLIST_FOREACH(so , &qiv->stack, node) {
-        if (qobject_type(so->obj) == QTYPE_QDICT) {
-            g_string_prepend(qiv->errname, name);
+        if (n) {
+            n--;
+        } else if (qobject_type(so->obj) == QTYPE_QDICT) {
+            g_string_prepend(qiv->errname, name ?: "<anonymous>");
             g_string_prepend_c(qiv->errname, '.');
         } else {
             snprintf(buf, sizeof(buf), "[%u]", so->index);
@@ -72,18 +75,24 @@ static const char *full_name(QObjectInputVisitor *qiv, const char *name)
         }
         name = so->name;
     }
+    assert(!n);
 
     if (name) {
         g_string_prepend(qiv->errname, name);
     } else if (qiv->errname->str[0] == '.') {
         g_string_erase(qiv->errname, 0, 1);
-    } else {
+    } else if (!qiv->errname->str[0]) {
         return "<anonymous>";
     }
 
     return qiv->errname->str;
 }
 
+static const char *full_name(QObjectInputVisitor *qiv, const char *name)
+{
+    return full_name_nth(qiv, name, 0);
+}
+
 static QObject *qobject_input_try_get_object(QObjectInputVisitor *qiv,
                                              const char *name,
                                              bool consume)
@@ -260,15 +269,30 @@ static GenericList *qobject_input_next_list(Visitor *v, GenericList *tail,
                                             size_t size)
 {
     QObjectInputVisitor *qiv = to_qiv(v);
-    StackObject *so = QSLIST_FIRST(&qiv->stack);
+    StackObject *tos = QSLIST_FIRST(&qiv->stack);
+
+    assert(tos && tos->obj && qobject_type(tos->obj) == QTYPE_QLIST);
 
-    if (!so->entry) {
+    if (!tos->entry) {
         return NULL;
     }
     tail->next = g_malloc0(size);
     return tail->next;
 }
 
+static void qobject_input_check_list(Visitor *v, Error **errp)
+{
+    QObjectInputVisitor *qiv = to_qiv(v);
+    StackObject *tos = QSLIST_FIRST(&qiv->stack);
+
+    assert(tos && tos->obj && qobject_type(tos->obj) == QTYPE_QLIST);
+
+    if (tos->entry) {
+        error_setg(errp, "Only %u list elements expected in %s",
+                   tos->index + 1, full_name_nth(qiv, NULL, 1));
+    }
+}
+
 
 static void qobject_input_start_alternate(Visitor *v, const char *name,
                                           GenericAlternate **obj, size_t size,
@@ -471,6 +495,7 @@ Visitor *qobject_input_visitor_new(QObject *obj)
     v->visitor.end_struct = qobject_input_pop;
     v->visitor.start_list = qobject_input_start_list;
     v->visitor.next_list = qobject_input_next_list;
+    v->visitor.check_list = qobject_input_check_list;
     v->visitor.end_list = qobject_input_pop;
     v->visitor.start_alternate = qobject_input_start_alternate;
     v->visitor.type_int64 = qobject_input_type_int64;
diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
index f126cd95a9..806b01ae3a 100644
--- a/qapi/string-input-visitor.c
+++ b/qapi/string-input-visitor.c
@@ -170,6 +170,35 @@ static GenericList *next_list(Visitor *v, GenericList *tail, size_t size)
     return tail->next;
 }
 
+static void check_list(Visitor *v, Error **errp)
+{
+    const StringInputVisitor *siv = to_siv(v);
+    Range *r;
+    GList *cur_range;
+
+    if (!siv->ranges || !siv->cur_range) {
+        return;
+    }
+
+    r = siv->cur_range->data;
+    if (!r) {
+        return;
+    }
+
+    if (!range_contains(r, siv->cur)) {
+        cur_range = g_list_next(siv->cur_range);
+        if (!cur_range) {
+            return;
+        }
+        r = cur_range->data;
+        if (!r) {
+            return;
+        }
+    }
+
+    error_setg(errp, "Range contains too many values");
+}
+
 static void end_list(Visitor *v, void **obj)
 {
     StringInputVisitor *siv = to_siv(v);
@@ -318,6 +347,7 @@ Visitor *string_input_visitor_new(const char *str)
     v->visitor.type_number = parse_type_number;
     v->visitor.start_list = start_list;
     v->visitor.next_list = next_list;
+    v->visitor.check_list = check_list;
     v->visitor.end_list = end_list;
     v->visitor.free = string_input_free;
 
diff --git a/qapi/trace-events b/qapi/trace-events
index 9cbb61b2bd..339cacf0ad 100644
--- a/qapi/trace-events
+++ b/qapi/trace-events
@@ -8,6 +8,7 @@ visit_end_struct(void *v, void *obj) "v=%p obj=%p"
 
 visit_start_list(void *v, const char *name, void *obj, size_t size) "v=%p name=%s obj=%p size=%zu"
 visit_next_list(void *v, void *tail, size_t size) "v=%p tail=%p size=%zu"
+visit_check_list(void *v) "v=%p"
 visit_end_list(void *v, void *obj) "v=%p obj=%p"
 
 visit_start_alternate(void *v, const char *name, void *obj, size_t size, bool promote_int) "v=%p name=%s obj=%p size=%zu promote_int=%d"
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 96f2491c16..330b9f321b 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -133,6 +133,9 @@ void visit_type_%(c_name)s(Visitor *v, const char *name, %(c_name)s **obj, Error
         }
     }
 
+    if (!err) {
+        visit_check_list(v, &err);
+    }
     visit_end_list(v, (void **)obj);
     if (err && visit_is_input(v)) {
         qapi_free_%(c_name)s(*obj);
diff --git a/tests/test-opts-visitor.c b/tests/test-opts-visitor.c
index d0f764699b..b93fd330a8 100644
--- a/tests/test-opts-visitor.c
+++ b/tests/test-opts-visitor.c
@@ -199,8 +199,8 @@ test_opts_range_unvisited(void)
     g_assert_cmpint(tail->value, ==, 1);
     tail = (intList *)visit_next_list(v, (GenericList *)tail, sizeof(*list));
     g_assert(tail);
+    visit_check_list(v, &error_abort); /* BUG: unvisited tail not reported */
     visit_end_list(v, (void **)&list);
-    /* BUG: unvisited tail not reported; actually not reportable by design */
 
     visit_check_struct(v, &error_abort);
     visit_end_struct(v, NULL);
diff --git a/tests/test-qobject-input-visitor.c b/tests/test-qobject-input-visitor.c
index 9f3a826353..87d4a77e4a 100644
--- a/tests/test-qobject-input-visitor.c
+++ b/tests/test-qobject-input-visitor.c
@@ -933,6 +933,7 @@ static void test_visitor_in_fail_list(TestInputVisitorData *data,
                                       const void *unused)
 {
     int64_t i64 = -1;
+    Error *err = NULL;
     Visitor *v;
 
     /* Unvisited list tail */
@@ -944,14 +945,16 @@ static void test_visitor_in_fail_list(TestInputVisitorData *data,
     g_assert_cmpint(i64, ==, 1);
     visit_type_int(v, NULL, &i64, &error_abort);
     g_assert_cmpint(i64, ==, 2);
+    visit_check_list(v, &err);
+    error_free_or_abort(&err);
     visit_end_list(v, NULL);
-    /* BUG: unvisited tail not reported; actually not reportable by design */
 }
 
 static void test_visitor_in_fail_list_nested(TestInputVisitorData *data,
                                              const void *unused)
 {
     int64_t i64 = -1;
+    Error *err = NULL;
     Visitor *v;
 
     /* Unvisited nested list tail */
@@ -964,8 +967,10 @@ static void test_visitor_in_fail_list_nested(TestInputVisitorData *data,
     visit_start_list(v, NULL, NULL, 0, &error_abort);
     visit_type_int(v, NULL, &i64, &error_abort);
     g_assert_cmpint(i64, ==, 1);
+    visit_check_list(v, &err);
+    error_free_or_abort(&err);
     visit_end_list(v, NULL);
-    /* BUG: unvisited tail not reported; actually not reportable by design */
+    visit_check_list(v, &error_abort);
     visit_end_list(v, NULL);
 }
 
diff --git a/tests/test-string-input-visitor.c b/tests/test-string-input-visitor.c
index 70cee65cd0..fbe380acbe 100644
--- a/tests/test-string-input-visitor.c
+++ b/tests/test-string-input-visitor.c
@@ -169,8 +169,10 @@ static void test_visitor_in_intList(TestInputVisitorData *data,
     g_assert_cmpint(tail->value, ==, 2);
     tail = (int64List *)visit_next_list(v, (GenericList *)tail, sizeof(*res));
     g_assert(tail);
+
+    visit_check_list(v, &err);
+    error_free_or_abort(&err);
     visit_end_list(v, (void **)&res);
-    /* BUG: unvisited tail not reported; actually not reportable by design */
 
     qapi_free_int64List(res);
 }
-- 
cgit 1.4.1


From 1f41a645b65530859bf5984aa08e103bb452b473 Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Fri, 3 Mar 2017 13:32:47 +0100
Subject: qapi: Fix object input visit beyond end of list

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1488544368-30622-28-git-send-email-armbru@redhat.com>
---
 qapi/qobject-input-visitor.c       | 11 ++++++++---
 tests/test-qobject-input-visitor.c |  2 --
 2 files changed, 8 insertions(+), 5 deletions(-)

(limited to 'qapi/qobject-input-visitor.c')

diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
index 34065ba7dd..d192727e0b 100644
--- a/qapi/qobject-input-visitor.c
+++ b/qapi/qobject-input-visitor.c
@@ -122,10 +122,15 @@ static QObject *qobject_input_try_get_object(QObjectInputVisitor *qiv,
     } else {
         assert(qobject_type(qobj) == QTYPE_QLIST);
         assert(!name);
-        ret = qlist_entry_obj(tos->entry);
-        assert(ret);
+        if (tos->entry) {
+            ret = qlist_entry_obj(tos->entry);
+            if (consume) {
+                tos->entry = qlist_next(tos->entry);
+            }
+        } else {
+            ret = NULL;
+        }
         if (consume) {
-            tos->entry = qlist_next(tos->entry);
             tos->index++;
         }
     }
diff --git a/tests/test-qobject-input-visitor.c b/tests/test-qobject-input-visitor.c
index 8011baaa38..94305f58ca 100644
--- a/tests/test-qobject-input-visitor.c
+++ b/tests/test-qobject-input-visitor.c
@@ -953,10 +953,8 @@ static void test_visitor_in_fail_list(TestInputVisitorData *data,
     v = visitor_input_test_init(data, "[]");
 
     visit_start_list(v, NULL, NULL, 0, &error_abort);
-#if 0 /* FIXME crash */
     visit_type_int(v, NULL, &i64, &err);
     error_free_or_abort(&err);
-#endif
     visit_end_list(v, NULL);
 }
 
-- 
cgit 1.4.1