Merge remote-tracking branch 'remotes/armbru/tags/pull-qapi-2016-05-12' into staging

QAPI patches for 2016-05-12

# gpg: Signature made Thu 12 May 2016 08:49:04 BST using RSA key ID EB918653
# gpg: Good signature from "Markus Armbruster <armbru@redhat.com>"
# gpg:                 aka "Markus Armbruster <armbru@pond.sub.org>"

* remotes/armbru/tags/pull-qapi-2016-05-12: (23 commits)
  qapi: Change visit_type_FOO() to no longer return partial objects
  qapi: Simplify semantics of visit_next_list()
  qapi: Fix string input visitor handling of invalid list
  tests/string-input-visitor: Add negative integer tests
  qapi: Split visit_end_struct() into pieces
  qmp: Tighten output visitor rules
  qmp: Don't reuse qmp visitor after grabbing output
  spapr_drc: Expose 'null' in qom-get when there is no fdt
  qmp: Support explicit null during visits
  qapi: Add visit_type_null() visitor
  tests: Add check-qnull
  qapi: Document visitor interfaces, add assertions
  qmp-input: Refactor when list is advanced
  qmp-input: Require struct push to visit members of top dict
  qom: Wrap prop visit in visit_start_struct
  qapi-commands: Wrap argument visit in visit_start_struct
  qmp-input: Don't consume input when checking has_member
  qapi: Use strict QMP input visitor in more places
  qapi: Consolidate QMP input visitor creation
  qmp-input: Clean up stack handling
  ...
Signed-off-by: NPeter Maydell <peter.maydell@linaro.org>

block/crypto.c

@@ -196,7 +196,6 @@ block_crypto_open_opts_init(QCryptoBlockFormat format,
     OptsVisitor *ov;
     QCryptoBlockOpenOptions *ret = NULL;
     Error *local_err = NULL;
-    Error *end_err = NULL;
 
     ret = g_new0(QCryptoBlockOpenOptions, 1);
     ret->format = format;
@@ -219,9 +218,11 @@ block_crypto_open_opts_init(QCryptoBlockFormat format,
         error_setg(&local_err, "Unsupported block format %d", format);
         break;
     }
+    if (!local_err) {
+        visit_check_struct(opts_get_visitor(ov), &local_err);
+    }
 
-    visit_end_struct(opts_get_visitor(ov), &end_err);
-    error_propagate(&local_err, end_err);
+    visit_end_struct(opts_get_visitor(ov));
 
  out:
     if (local_err) {
@@ -242,7 +243,6 @@ block_crypto_create_opts_init(QCryptoBlockFormat format,
     OptsVisitor *ov;
     QCryptoBlockCreateOptions *ret = NULL;
     Error *local_err = NULL;
-    Error *end_err = NULL;
 
     ret = g_new0(QCryptoBlockCreateOptions, 1);
     ret->format = format;
@@ -265,9 +265,11 @@ block_crypto_create_opts_init(QCryptoBlockFormat format,
         error_setg(&local_err, "Unsupported block format %d", format);
         break;
     }
+    if (!local_err) {
+        visit_check_struct(opts_get_visitor(ov), &local_err);
+    }
 
-    visit_end_struct(opts_get_visitor(ov), &end_err);
-    error_propagate(&local_err, end_err);
+    visit_end_struct(opts_get_visitor(ov));
 
  out:
     if (local_err) {

docs/qapi-code-gen.txt

@@ -899,10 +899,16 @@ Example:
             goto out_obj;
         }
         visit_type_UserDefOne_members(v, *obj, &err);
-        error_propagate(errp, err);
-        err = NULL;
+        if (err) {
+            goto out_obj;
+        }
+        visit_check_struct(v, &err);
     out_obj:
-        visit_end_struct(v, &err);
+        visit_end_struct(v);
+        if (err && visit_is_input(v)) {
+            qapi_free_UserDefOne(*obj);
+            *obj = NULL;
+        }
     out:
         error_propagate(errp, err);
     }
@@ -910,21 +916,27 @@ Example:
     void visit_type_UserDefOneList(Visitor *v, const char *name, UserDefOneList **obj, Error **errp)
     {
         Error *err = NULL;
-        GenericList *i, **prev;
+        UserDefOneList *tail;
+        size_t size = sizeof(**obj);
 
-        visit_start_list(v, name, &err);
+        visit_start_list(v, name, (GenericList **)obj, size, &err);
         if (err) {
             goto out;
         }
 
-        for (prev = (GenericList **)obj;
-             !err && (i = visit_next_list(v, prev, sizeof(**obj))) != NULL;
-             prev = &i) {
-            UserDefOneList *native_i = (UserDefOneList *)i;
-            visit_type_UserDefOne(v, NULL, &native_i->value, &err);
+        for (tail = *obj; tail;
+             tail = (UserDefOneList *)visit_next_list(v, (GenericList *)tail, size)) {
+            visit_type_UserDefOne(v, NULL, &tail->value, &err);
+            if (err) {
+                break;
+            }
         }
 
         visit_end_list(v);
+        if (err && visit_is_input(v)) {
+            qapi_free_UserDefOneList(*obj);
+            *obj = NULL;
+        }
     out:
         error_propagate(errp, err);
     }
@@ -996,13 +1008,21 @@ Example:
     {
         Error *err = NULL;
         UserDefOne *retval;
-        QmpInputVisitor *qiv = qmp_input_visitor_new_strict(QOBJECT(args));
+        QmpInputVisitor *qiv = qmp_input_visitor_new(QOBJECT(args), true);
         QapiDeallocVisitor *qdv;
         Visitor *v;
         UserDefOneList *arg1 = NULL;
 
         v = qmp_input_get_visitor(qiv);
+        visit_start_struct(v, NULL, NULL, 0, &err);
+        if (err) {
+            goto out;
+        }
         visit_type_UserDefOneList(v, "arg1", &arg1, &err);
+        if (!err) {
+            visit_check_struct(v, &err);
+        }
+        visit_end_struct(v);
         if (err) {
             goto out;
         }
@@ -1019,7 +1039,9 @@ Example:
         qmp_input_visitor_cleanup(qiv);
         qdv = qapi_dealloc_visitor_new();
         v = qapi_dealloc_get_visitor(qdv);
+        visit_start_struct(v, NULL, NULL, 0, NULL);
         visit_type_UserDefOneList(v, "arg1", &arg1, NULL);
+        visit_end_struct(v);
         qapi_dealloc_visitor_cleanup(qdv);
     }
 

hw/ppc/spapr_drc.c

@@ -269,11 +269,7 @@ static void prop_get_fdt(Object *obj, Visitor *v, const char *name,
     void *fdt;
 
     if (!drc->fdt) {
-        visit_start_struct(v, name, NULL, 0, &err);
-        if (!err) {
-            visit_end_struct(v, &err);
-        }
-        error_propagate(errp, err);
+        visit_type_null(v, NULL, errp);
         return;
     }
 
@@ -301,7 +297,8 @@ static void prop_get_fdt(Object *obj, Visitor *v, const char *name,
         case FDT_END_NODE:
             /* shouldn't ever see an FDT_END_NODE before FDT_BEGIN_NODE */
             g_assert(fdt_depth > 0);
-            visit_end_struct(v, &err);
+            visit_check_struct(v, &err);
+            visit_end_struct(v);
             if (err) {
                 error_propagate(errp, err);
                 return;
@@ -312,7 +309,7 @@ static void prop_get_fdt(Object *obj, Visitor *v, const char *name,
             int i;
             prop = fdt_get_property_by_offset(fdt, fdt_offset, &prop_len);
             name = fdt_string(fdt, fdt32_to_cpu(prop->nameoff));
-            visit_start_list(v, name, &err);
+            visit_start_list(v, name, NULL, 0, &err);
             if (err) {
                 error_propagate(errp, err);
                 return;

hw/virtio/virtio-balloon.c

@@ -138,17 +138,18 @@ static void balloon_stats_get_all(Object *obj, Visitor *v, const char *name,
     for (i = 0; i < VIRTIO_BALLOON_S_NR; i++) {
         visit_type_uint64(v, balloon_stat_names[i], &s->stats[i], &err);
         if (err) {
-            break;
+            goto out_nested;
         }
     }
-    error_propagate(errp, err);
-    err = NULL;
-    visit_end_struct(v, &err);
+    visit_check_struct(v, &err);
+out_nested:
+    visit_end_struct(v);
 
+    if (!err) {
+        visit_check_struct(v, &err);
+    }
 out_end:
-    error_propagate(errp, err);
-    err = NULL;
-    visit_end_struct(v, &err);
+    visit_end_struct(v);
 out:
     error_propagate(errp, err);
 }

include/qapi/dealloc-visitor.h

@@ -18,6 +18,11 @@
 
 typedef struct QapiDeallocVisitor QapiDeallocVisitor;
 
+/*
+ * The dealloc visitor is primarly used only by generated
+ * qapi_free_FOO() functions, and is the only visitor designed to work
+ * correctly in the face of a partially-constructed QAPI tree.
+ */
 QapiDeallocVisitor *qapi_dealloc_visitor_new(void);
 void qapi_dealloc_visitor_cleanup(QapiDeallocVisitor *d);
 

include/qapi/opts-visitor.h

@@ -29,6 +29,11 @@ typedef struct OptsVisitor OptsVisitor;
  * - string representations of negative numbers yield negative values,
  * - values below INT64_MIN or LLONG_MIN are rejected,
  * - values above INT64_MAX or LLONG_MAX are rejected.
+ *
+ * The Opts input visitor does not implement support for visiting QAPI
+ * alternates, numbers (other than integers), null, or arbitrary
+ * QTypes.  It also requires a non-null list argument to
+ * visit_start_list().
  */
 OptsVisitor *opts_visitor_new(const QemuOpts *opts);
 void opts_visitor_cleanup(OptsVisitor *nv);

include/qapi/qmp-input-visitor.h

@@ -19,8 +19,13 @@
 
 typedef struct QmpInputVisitor QmpInputVisitor;
 
-QmpInputVisitor *qmp_input_visitor_new(QObject *obj);
-QmpInputVisitor *qmp_input_visitor_new_strict(QObject *obj);
+/*
+ * Return a new input visitor that converts QMP to QAPI.
+ *
+ * Set @strict to reject a parse that doesn't consume all keys of a
+ * dictionary; otherwise excess input is ignored.
+ */
+QmpInputVisitor *qmp_input_visitor_new(QObject *obj, bool strict);
 
 void qmp_input_visitor_cleanup(QmpInputVisitor *v);
 

include/qapi/qmp/dispatch.h

@@ -19,11 +19,6 @@
 
 typedef void (QmpCommandFunc)(QDict *, QObject **, Error **);
 
-typedef enum QmpCommandType
-{
-    QCT_NORMAL,
-} QmpCommandType;
-
 typedef enum QmpCommandOptions
 {
     QCO_NO_OPTIONS = 0x0,
@@ -33,7 +28,6 @@ typedef enum QmpCommandOptions
 typedef struct QmpCommand
 {
     const char *name;
-    QmpCommandType type;
     QmpCommandFunc *fn;
     QmpCommandOptions options;
     QTAILQ_ENTRY(QmpCommand) node;

include/qapi/string-input-visitor.h

@@ -17,6 +17,11 @@
 
 typedef struct StringInputVisitor StringInputVisitor;
 
+/*
+ * The string input visitor does not implement support for visiting
+ * QAPI structs, alternates, null, or arbitrary QTypes.  It also
+ * requires a non-null list argument to visit_start_list().
+ */
 StringInputVisitor *string_input_visitor_new(const char *str);
 void string_input_visitor_cleanup(StringInputVisitor *v);
 

include/qapi/string-output-visitor.h

@@ -17,6 +17,11 @@
 
 typedef struct StringOutputVisitor StringOutputVisitor;
 
+/*
+ * The string output visitor does not implement support for visiting
+ * QAPI structs, alternates, null, or arbitrary QTypes.  It also
+ * requires a non-null list argument to visit_start_list().
+ */
 StringOutputVisitor *string_output_visitor_new(bool human);
 void string_output_visitor_cleanup(StringOutputVisitor *v);
 

include/qapi/visitor-impl.h

@@ -14,55 +14,96 @@
 
 #include "qapi/visitor.h"
 
+/*
+ * This file describes the callback interface for implementing a QAPI
+ * visitor.  For the client interface, see visitor.h.  When
+ * implementing the callbacks, it is easiest to declare a struct with
+ * 'Visitor visitor;' as the first member.  A callback's contract
+ * matches the corresponding public functions' contract unless stated
+ * otherwise.  In the comments below, some callbacks are marked "must
+ * be set for $TYPE visits to work"; if a visitor implementation omits
+ * that callback, it should also document that it is only useful for a
+ * subset of QAPI.
+ */
+
+/*
+ * There are three classes of visitors; setting the class determines
+ * how QAPI enums are visited, as well as what additional restrictions
+ * can be asserted.
+ */
+typedef enum VisitorType {
+    VISITOR_INPUT,
+    VISITOR_OUTPUT,
+    VISITOR_DEALLOC,
+} VisitorType;
+
 struct Visitor
 {
-    /* Must be set */
+    /* Must be set to visit structs */
     void (*start_struct)(Visitor *v, const char *name, void **obj,
                          size_t size, Error **errp);
-    void (*end_struct)(Visitor *v, Error **errp);
 
-    void (*start_list)(Visitor *v, const char *name, Error **errp);
+    /* Optional; intended for input visitors */
+    void (*check_struct)(Visitor *v, Error **errp);
+
+    /* Must be set to visit structs */
+    void (*end_struct)(Visitor *v);
+
+    /* Must be set; implementations may require @list to be non-null,
+     * but must document it. */
+    void (*start_list)(Visitor *v, const char *name, GenericList **list,
+                       size_t size, Error **errp);
+
     /* Must be set */
-    GenericList *(*next_list)(Visitor *v, GenericList **list, size_t size);
+    GenericList *(*next_list)(Visitor *v, GenericList *tail, size_t size);
+
     /* Must be set */
     void (*end_list)(Visitor *v);
 
-    /* Optional, needed for input and dealloc visitors.  */
+    /* Must be set by input and dealloc visitors to visit alternates;
+     * optional for output visitors. */
     void (*start_alternate)(Visitor *v, const char *name,
                             GenericAlternate **obj, size_t size,
                             bool promote_int, Error **errp);
 
-    /* Optional, needed for dealloc visitor.  */
+    /* Optional, needed for dealloc visitor */
     void (*end_alternate)(Visitor *v);
 
-    /* Must be set. */
-    void (*type_enum)(Visitor *v, const char *name, int *obj,
-                      const char *const strings[], Error **errp);
-
-    /* Must be set. */
+    /* Must be set */
     void (*type_int64)(Visitor *v, const char *name, int64_t *obj,
                        Error **errp);
-    /* Must be set. */
+
+    /* Must be set */
     void (*type_uint64)(Visitor *v, const char *name, uint64_t *obj,
                         Error **errp);
-    /* Optional; fallback is type_uint64().  */
+
+    /* Optional; fallback is type_uint64() */
     void (*type_size)(Visitor *v, const char *name, uint64_t *obj,
                       Error **errp);
-    /* Must be set. */
+
+    /* Must be set */
     void (*type_bool)(Visitor *v, const char *name, bool *obj, Error **errp);
+
+    /* Must be set */
     void (*type_str)(Visitor *v, const char *name, char **obj, Error **errp);
+
+    /* Must be set to visit numbers */
     void (*type_number)(Visitor *v, const char *name, double *obj,
                         Error **errp);
+
+    /* Must be set to visit arbitrary QTypes */
     void (*type_any)(Visitor *v, const char *name, QObject **obj,
                      Error **errp);
 
-    /* May be NULL; most useful for input visitors. */
+    /* Must be set to visit explicit null values.  */
+    void (*type_null)(Visitor *v, const char *name, Error **errp);
+
+    /* Must be set for input visitors, optional otherwise.  The core
+     * takes care of the return type in the public interface. */
     void (*optional)(Visitor *v, const char *name, bool *present);
-};
 
-void input_type_enum(Visitor *v, const char *name, int *obj,
-                     const char *const strings[], Error **errp);
-void output_type_enum(Visitor *v, const char *name, int *obj,
-                      const char *const strings[], Error **errp);
+    /* Must be set */
+    VisitorType type;
+};
 
 #endif

qapi/opts-visitor.c

@@ -23,9 +23,8 @@
 enum ListMode
 {
     LM_NONE,             /* not traversing a list of repeated options */
-    LM_STARTED,          /* opts_start_list() succeeded */
 
-    LM_IN_PROGRESS,      /* opts_next_list() has been called.
+    LM_IN_PROGRESS,      /* opts_next_list() ready to be called.
                           *
                           * Generating the next list link will consume the most
                           * recently parsed QemuOpt instance of the repeated
@@ -133,7 +132,7 @@ opts_start_struct(Visitor *v, const char *name, void **obj,
     const QemuOpt *opt;
 
     if (obj) {
-        *obj = g_malloc0(size > 0 ? size : 1);
+        *obj = g_malloc0(size);
     }
     if (ov->depth++ > 0) {
         return;
@@ -159,13 +158,13 @@ opts_start_struct(Visitor *v, const char *name, void **obj,
 
 
 static void
-opts_end_struct(Visitor *v, Error **errp)
+opts_check_struct(Visitor *v, Error **errp)
 {
     OptsVisitor *ov = to_ov(v);
     GHashTableIter iter;
     GQueue *any;
 
-    if (--ov->depth > 0) {
+    if (ov->depth > 0) {
         return;
     }
 
@@ -177,6 +176,18 @@ opts_end_struct(Visitor *v, Error **errp)
         first = g_queue_peek_head(any);
         error_setg(errp, QERR_INVALID_PARAMETER, first->name);
     }
+}
+
+
+static void
+opts_end_struct(Visitor *v)
+{
+    OptsVisitor *ov = to_ov(v);
+
+    if (--ov->depth > 0) {
+        return;
+    }
+
     g_hash_table_destroy(ov->unprocessed_opts);
     ov->unprocessed_opts = NULL;
     if (ov->fake_id_opt) {
@@ -202,35 +213,33 @@ lookup_distinct(const OptsVisitor *ov, const char *name, Error **errp)
 
 
 static void
-opts_start_list(Visitor *v, const char *name, Error **errp)
+opts_start_list(Visitor *v, const char *name, GenericList **list, size_t size,
+                Error **errp)
 {
     OptsVisitor *ov = to_ov(v);
 
     /* we can't traverse a list in a list */
     assert(ov->list_mode == LM_NONE);
+    /* we don't support visits without a list */
+    assert(list);
     ov->repeated_opts = lookup_distinct(ov, name, errp);
-    if (ov->repeated_opts != NULL) {
-        ov->list_mode = LM_STARTED;
+    if (ov->repeated_opts) {
+        ov->list_mode = LM_IN_PROGRESS;
+        *list = g_malloc0(size);
+    } else {
+        *list = NULL;
     }
 }
 
 
 static GenericList *
-opts_next_list(Visitor *v, GenericList **list, size_t size)
+opts_next_list(Visitor *v, GenericList *tail, size_t size)
 {
     OptsVisitor *ov = to_ov(v);
-    GenericList **link;
 
     switch (ov->list_mode) {
-    case LM_STARTED:
-        ov->list_mode = LM_IN_PROGRESS;
-        link = list;
-        break;
-
     case LM_SIGNED_INTERVAL:
     case LM_UNSIGNED_INTERVAL:
-        link = &(*list)->next;
-
         if (ov->list_mode == LM_SIGNED_INTERVAL) {
             if (ov->range_next.s < ov->range_limit.s) {
                 ++ov->range_next.s;
@@ -251,7 +260,6 @@ opts_next_list(Visitor *v, GenericList **list, size_t size)
             g_hash_table_remove(ov->unprocessed_opts, opt->name);
             return NULL;
         }
-        link = &(*list)->next;
         break;
     }
 
@@ -259,8 +267,8 @@ opts_next_list(Visitor *v, GenericList **list, size_t size)
         abort();
     }
 
-    *link = g_malloc0(size);
-    return *link;
+    tail->next = g_malloc0(size);
+    return tail->next;
 }
 
 
@@ -269,8 +277,7 @@ opts_end_list(Visitor *v)
 {
     OptsVisitor *ov = to_ov(v);
 
-    assert(ov->list_mode == LM_STARTED ||
-           ov->list_mode == LM_IN_PROGRESS ||
+    assert(ov->list_mode == LM_IN_PROGRESS ||
            ov->list_mode == LM_SIGNED_INTERVAL ||
            ov->list_mode == LM_UNSIGNED_INTERVAL);
     ov->repeated_opts = NULL;
@@ -314,9 +321,15 @@ opts_type_str(Visitor *v, const char *name, char **obj, Error **errp)
 
     opt = lookup_scalar(ov, name, errp);
     if (!opt) {
+        *obj = NULL;
         return;
     }
     *obj = g_strdup(opt->str ? opt->str : "");
+    /* Note that we consume a string even if this is called as part of
+     * an enum visit that later fails because the string is not a
+     * valid enum value; this is harmless because tracking what gets
+     * consumed only matters to visit_end_struct() as the final error
+     * check if there were no other failures during the visit.  */
     processed(ov, name);
 }
 
@@ -507,23 +520,16 @@ opts_visitor_new(const QemuOpts *opts)
 
     ov = g_malloc0(sizeof *ov);
 
+    ov->visitor.type = VISITOR_INPUT;
+
     ov->visitor.start_struct = &opts_start_struct;
+    ov->visitor.check_struct = &opts_check_struct;
     ov->visitor.end_struct   = &opts_end_struct;
 
     ov->visitor.start_list = &opts_start_list;
     ov->visitor.next_list  = &opts_next_list;
     ov->visitor.end_list   = &opts_end_list;
 
-    /* input_type_enum() covers both "normal" enums and union discriminators.
-     * The union discriminator field is always generated as "type"; it should
-     * match the "type" QemuOpt child of any QemuOpts.
-     *
-     * input_type_enum() will remove the looked-up key from the
-     * "unprocessed_opts" hash even if the lookup fails, because the removal is
-     * done earlier in opts_type_str(). This should be harmless.
-     */
-    ov->visitor.type_enum = &input_type_enum;
-
     ov->visitor.type_int64  = &opts_type_int64;
     ov->visitor.type_uint64 = &opts_type_uint64;
     ov->visitor.type_size   = &opts_type_size;

qapi/qapi-dealloc-visitor.c

@@ -22,7 +22,6 @@
 typedef struct StackEntry
 {
     void *value;
-    bool is_list_head;
     QTAILQ_ENTRY(StackEntry) node;
 } StackEntry;
 
@@ -43,10 +42,6 @@ static void qapi_dealloc_push(QapiDeallocVisitor *qov, void *value)
 
     e->value = value;
 
-    /* see if we're just pushing a list head tracker */
-    if (value == NULL) {
-        e->is_list_head = true;
-    }
     QTAILQ_INSERT_HEAD(&qov->stack, e, node);
 }
 
@@ -67,7 +62,7 @@ static void qapi_dealloc_start_struct(Visitor *v, const char *name, void **obj,
     qapi_dealloc_push(qov, obj);
 }
 
-static void qapi_dealloc_end_struct(Visitor *v, Error **errp)
+static void qapi_dealloc_end_struct(Visitor *v)
 {
     QapiDeallocVisitor *qov = to_qov(v);
     void **obj = qapi_dealloc_pop(qov);
@@ -93,38 +88,22 @@ static void qapi_dealloc_end_alternate(Visitor *v)
     }
 }
 
-static void qapi_dealloc_start_list(Visitor *v, const char *name, Error **errp)
+static void qapi_dealloc_start_list(Visitor *v, const char *name,
+                                    GenericList **list, size_t size,
+                                    Error **errp)
 {
-    QapiDeallocVisitor *qov = to_qov(v);
-    qapi_dealloc_push(qov, NULL);
 }
 
-static GenericList *qapi_dealloc_next_list(Visitor *v, GenericList **listp,
+static GenericList *qapi_dealloc_next_list(Visitor *v, GenericList *tail,
                                            size_t size)
 {
-    GenericList *list = *listp;
-    QapiDeallocVisitor *qov = to_qov(v);
-    StackEntry *e = QTAILQ_FIRST(&qov->stack);
-
-    if (e && e->is_list_head) {
-        e->is_list_head = false;
-        return list;
-    }
-
-    if (list) {
-        list = list->next;
-        g_free(*listp);
-        return list;
-    }
-
-    return NULL;
+    GenericList *next = tail->next;
+    g_free(tail);
+    return next;
 }
 
 static void qapi_dealloc_end_list(Visitor *v)
 {
-    QapiDeallocVisitor *qov = to_qov(v);
-    void *obj = qapi_dealloc_pop(qov);
-    assert(obj == NULL); /* should've been list head tracker with no payload */
 }
 
 static void qapi_dealloc_type_str(Visitor *v, const char *name, char **obj,
@@ -163,8 +142,7 @@ static void qapi_dealloc_type_anything(Visitor *v, const char *name,
     }
 }
 
-static void qapi_dealloc_type_enum(Visitor *v, const char *name, int *obj,
-                                   const char * const strings[], Error **errp)
+static void qapi_dealloc_type_null(Visitor *v, const char *name, Error **errp)
 {
 }
 
@@ -184,6 +162,7 @@ QapiDeallocVisitor *qapi_dealloc_visitor_new(void)
 
     v = g_malloc0(sizeof(*v));
 
+    v->visitor.type = VISITOR_DEALLOC;
     v->visitor.start_struct = qapi_dealloc_start_struct;
     v->visitor.end_struct = qapi_dealloc_end_struct;
     v->visitor.start_alternate = qapi_dealloc_start_alternate;
@@ -191,13 +170,13 @@ QapiDeallocVisitor *qapi_dealloc_visitor_new(void)
     v->visitor.start_list = qapi_dealloc_start_list;
     v->visitor.next_list = qapi_dealloc_next_list;
     v->visitor.end_list = qapi_dealloc_end_list;
-    v->visitor.type_enum = qapi_dealloc_type_enum;
     v->visitor.type_int64 = qapi_dealloc_type_int64;
     v->visitor.type_uint64 = qapi_dealloc_type_uint64;
     v->visitor.type_bool = qapi_dealloc_type_bool;
     v->visitor.type_str = qapi_dealloc_type_str;
     v->visitor.type_number = qapi_dealloc_type_number;
     v->visitor.type_any = qapi_dealloc_type_anything;
+    v->visitor.type_null = qapi_dealloc_type_null;
 
     QTAILQ_INIT(&v->stack);
 

qapi/qapi-visit-core.c

@@ -23,23 +23,48 @@
 void visit_start_struct(Visitor *v, const char *name, void **obj,
                         size_t size, Error **errp)
 {
-    v->start_struct(v, name, obj, size, errp);
+    Error *err = NULL;
+
+    if (obj) {
+        assert(size);
+        assert(v->type != VISITOR_OUTPUT || *obj);
+    }
+    v->start_struct(v, name, obj, size, &err);
+    if (obj && v->type == VISITOR_INPUT) {
+        assert(!err != !*obj);
+    }
+    error_propagate(errp, err);
 }
 
-void visit_end_struct(Visitor *v, Error **errp)
+void visit_check_struct(Visitor *v, Error **errp)
 {
-    v->end_struct(v, errp);
+    if (v->check_struct) {
+        v->check_struct(v, errp);
+    }
 }
 
-void visit_start_list(Visitor *v, const char *name, Error **errp)
+void visit_end_struct(Visitor *v)
 {
-    v->start_list(v, name, errp);
+    v->end_struct(v);
 }
 
-GenericList *visit_next_list(Visitor *v, GenericList **list, size_t size)
+void visit_start_list(Visitor *v, const char *name, GenericList **list,
+                      size_t size, Error **errp)
 {
-    assert(list && size >= sizeof(GenericList));
-    return v->next_list(v, list, size);
+    Error *err = NULL;
+
+    assert(!list || size >= sizeof(GenericList));
+    v->start_list(v, name, list, size, &err);
+    if (list && v->type == VISITOR_INPUT) {
+        assert(!(err && *list));
+    }
+    error_propagate(errp, err);
+}
+
+GenericList *visit_next_list(Visitor *v, GenericList *tail, size_t size)
+{
+    assert(tail && size >= sizeof(GenericList));
+    return v->next_list(v, tail, size);
 }
 
 void visit_end_list(Visitor *v)
@@ -51,10 +76,17 @@ void visit_start_alternate(Visitor *v, const char *name,
                            GenericAlternate **obj, size_t size,
                            bool promote_int, Error **errp)
 {
+    Error *err = NULL;
+
     assert(obj && size >= sizeof(GenericAlternate));
+    assert(v->type != VISITOR_OUTPUT || *obj);
     if (v->start_alternate) {
-        v->start_alternate(v, name, obj, size, promote_int, errp);
+        v->start_alternate(v, name, obj, size, promote_int, &err);
     }
+    if (v->type == VISITOR_INPUT) {
+        assert(v->start_alternate && !err != !*obj);
+    }
+    error_propagate(errp, err);
 }
 
 void visit_end_alternate(Visitor *v)
@@ -72,14 +104,14 @@ bool visit_optional(Visitor *v, const char *name, bool *present)
     return *present;
 }
 
-void visit_type_enum(Visitor *v, const char *name, int *obj,
-                     const char *const strings[], Error **errp)
+bool visit_is_input(Visitor *v)
 {
-    v->type_enum(v, name, obj, strings, errp);
+    return v->type == VISITOR_INPUT;
 }
 
 void visit_type_int(Visitor *v, const char *name, int64_t *obj, Error **errp)
 {
+    assert(obj);
     v->type_int64(v, name, obj, errp);
 }
 
@@ -127,6 +159,7 @@ void visit_type_uint32(Visitor *v, const char *name, uint32_t *obj,
 void visit_type_uint64(Visitor *v, const char *name, uint64_t *obj,
                        Error **errp)
 {
+    assert(obj);
     v->type_uint64(v, name, obj, errp);
 }
 
@@ -174,12 +207,14 @@ void visit_type_int32(Visitor *v, const char *name, int32_t *obj,
 void visit_type_int64(Visitor *v, const char *name, int64_t *obj,
                       Error **errp)
 {
+    assert(obj);
     v->type_int64(v, name, obj, errp);
 }
 
 void visit_type_size(Visitor *v, const char *name, uint64_t *obj,
                      Error **errp)
 {
+    assert(obj);
     if (v->type_size) {
         v->type_size(v, name, obj, errp);
     } else {
@@ -189,33 +224,58 @@ void visit_type_size(Visitor *v, const char *name, uint64_t *obj,
 
 void visit_type_bool(Visitor *v, const char *name, bool *obj, Error **errp)
 {
+    assert(obj);
     v->type_bool(v, name, obj, errp);
 }
 
 void visit_type_str(Visitor *v, const char *name, char **obj, Error **errp)
 {
-    v->type_str(v, name, obj, errp);
+    Error *err = NULL;
+
+    assert(obj);
+    /* TODO: Fix callers to not pass NULL when they mean "", so that we
+     * can enable:
+    assert(v->type != VISITOR_OUTPUT || *obj);
+     */
+    v->type_str(v, name, obj, &err);
+    if (v->type == VISITOR_INPUT) {
+        assert(!err != !*obj);
+    }
+    error_propagate(errp, err);
 }
 
 void visit_type_number(Visitor *v, const char *name, double *obj,
                        Error **errp)
 {
+    assert(obj);
     v->type_number(v, name, obj, errp);
 }
 
 void visit_type_any(Visitor *v, const char *name, QObject **obj, Error **errp)
 {
-    v->type_any(v, name, obj, errp);
+    Error *err = NULL;
+
+    assert(obj);
+    assert(v->type != VISITOR_OUTPUT || *obj);
+    v->type_any(v, name, obj, &err);
+    if (v->type == VISITOR_INPUT) {
+        assert(!err != !*obj);
+    }
+    error_propagate(errp, err);
+}
+
+void visit_type_null(Visitor *v, const char *name, Error **errp)
+{
+    v->type_null(v, name, errp);
 }
 
-void output_type_enum(Visitor *v, const char *name, int *obj,
-                      const char *const strings[], Error **errp)
+static void output_type_enum(Visitor *v, const char *name, int *obj,
+                             const char *const strings[], Error **errp)
 {
     int i = 0;
     int value = *obj;
     char *enum_str;
 
-    assert(strings);
     while (strings[i++] != NULL);
     if (value < 0 || value >= i - 1) {
         error_setg(errp, QERR_INVALID_PARAMETER, name ? name : "null");
@@ -226,15 +286,13 @@ void output_type_enum(Visitor *v, const char *name, int *obj,
     visit_type_str(v, name, &enum_str, errp);
 }
 
-void input_type_enum(Visitor *v, const char *name, int *obj,
-                     const char *const strings[], Error **errp)
+static void input_type_enum(Visitor *v, const char *name, int *obj,
+                            const char *const strings[], Error **errp)
 {
     Error *local_err = NULL;
     int64_t value = 0;
     char *enum_str;
 
-    assert(strings);
-
     visit_type_str(v, name, &enum_str, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
@@ -257,3 +315,14 @@ void input_type_enum(Visitor *v, const char *name, int *obj,
     g_free(enum_str);
     *obj = value;
 }
+
+void visit_type_enum(Visitor *v, const char *name, int *obj,
+                     const char *const strings[], Error **errp)
+{
+    assert(obj && strings);
+    if (v->type == VISITOR_INPUT) {
+        input_type_enum(v, name, obj, strings, errp);
+    } else if (v->type == VISITOR_OUTPUT) {
+        output_type_enum(v, name, obj, strings, errp);
+    }
+}

qapi/qmp-dispatch.c

@@ -94,17 +94,13 @@ static QObject *do_qmp_dispatch(QObject *request, Error **errp)
         QINCREF(args);
     }
 
-    switch (cmd->type) {
-    case QCT_NORMAL:
-        cmd->fn(args, &ret, &local_err);
-        if (local_err) {
-            error_propagate(errp, local_err);
-        } else if (cmd->options & QCO_NO_SUCCESS_RESP) {
-            g_assert(!ret);
-        } else if (!ret) {
-            ret = QOBJECT(qdict_new());
-        }
-        break;
+    cmd->fn(args, &ret, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+    } else if (cmd->options & QCO_NO_SUCCESS_RESP) {
+        g_assert(!ret);
+    } else if (!ret) {
+        ret = QOBJECT(qdict_new());
     }
 
     QDECREF(args);

qapi/qmp-input-visitor.c

@@ -25,16 +25,25 @@
 
 typedef struct StackObject
 {
-    QObject *obj;
-    const QListEntry *entry;
-    GHashTable *h;
+    QObject *obj; /* Object being visited */
+
+    GHashTable *h;           /* If obj is dict: unvisited keys */
+    const QListEntry *entry; /* If obj is list: unvisited tail */
 } StackObject;
 
 struct QmpInputVisitor
 {
     Visitor visitor;
+
+    /* Root of visit at visitor creation. */
+    QObject *root;
+
+    /* Stack of objects being visited (all entries will be either
+     * QDict or QList). */
     StackObject stack[QIV_STACK_SIZE];
     int nb_stack;
+
+    /* True to reject parse in visit_end_struct() if unvisited keys remain. */
     bool strict;
 };
 
@@ -47,20 +56,37 @@ static QObject *qmp_input_get_object(QmpInputVisitor *qiv,
                                      const char *name,
                                      bool consume)
 {
-    QObject *qobj = qiv->stack[qiv->nb_stack - 1].obj;
+    StackObject *tos;
+    QObject *qobj;
+    QObject *ret;
 
-    if (qobj) {
-        if (name && qobject_type(qobj) == QTYPE_QDICT) {
-            if (qiv->stack[qiv->nb_stack - 1].h && consume) {
-                g_hash_table_remove(qiv->stack[qiv->nb_stack - 1].h, name);
-            }
-            return qdict_get(qobject_to_qdict(qobj), name);
-        } else if (qiv->stack[qiv->nb_stack - 1].entry) {
-            return qlist_entry_obj(qiv->stack[qiv->nb_stack - 1].entry);
+    if (!qiv->nb_stack) {
+        /* Starting at root, name is ignored. */
+        return qiv->root;
+    }
+
+    /* We are in a container; find the next element. */
+    tos = &qiv->stack[qiv->nb_stack - 1];
+    qobj = tos->obj;
+    assert(qobj);
+
+    if (qobject_type(qobj) == QTYPE_QDICT) {
+        assert(name);
+        ret = qdict_get(qobject_to_qdict(qobj), name);
+        if (tos->h && consume && ret) {
+            bool removed = g_hash_table_remove(tos->h, name);
+            assert(removed);
+        }
+    } else {
+        assert(qobject_type(qobj) == QTYPE_QLIST);
+        assert(!name);
+        ret = qlist_entry_obj(tos->entry);
+        if (consume) {
+            tos->entry = qlist_next(tos->entry);
         }
     }
 
-    return qobj;
+    return ret;
 }
 
 static void qdict_add_key(const char *key, QObject *obj, void *opaque)
@@ -69,35 +95,44 @@ static void qdict_add_key(const char *key, QObject *obj, void *opaque)
     g_hash_table_insert(h, (gpointer) key, NULL);
 }
 
-static void qmp_input_push(QmpInputVisitor *qiv, QObject *obj, Error **errp)
+static const QListEntry *qmp_input_push(QmpInputVisitor *qiv, QObject *obj,
+                                        Error **errp)
 {
     GHashTable *h;
+    StackObject *tos = &qiv->stack[qiv->nb_stack];
 
+    assert(obj);
     if (qiv->nb_stack >= QIV_STACK_SIZE) {
         error_setg(errp, "An internal buffer overran");
-        return;
+        return NULL;
     }
 
-    qiv->stack[qiv->nb_stack].obj = obj;
-    qiv->stack[qiv->nb_stack].entry = NULL;
-    qiv->stack[qiv->nb_stack].h = NULL;
+    tos->obj = obj;
+    assert(!tos->h);
+    assert(!tos->entry);
 
     if (qiv->strict && 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);
-        qiv->stack[qiv->nb_stack].h = h;
+        tos->h = h;
+    } else if (qobject_type(obj) == QTYPE_QLIST) {
+        tos->entry = qlist_first(qobject_to_qlist(obj));
     }
 
     qiv->nb_stack++;
+    return tos->entry;
 }
 
 
-static void qmp_input_pop(QmpInputVisitor *qiv, Error **errp)
+static void qmp_input_check_struct(Visitor *v, Error **errp)
 {
+    QmpInputVisitor *qiv = to_qiv(v);
+    StackObject *tos = &qiv->stack[qiv->nb_stack - 1];
+
     assert(qiv->nb_stack > 0);
 
     if (qiv->strict) {
-        GHashTable * const top_ht = qiv->stack[qiv->nb_stack - 1].h;
+        GHashTable *const top_ht = tos->h;
         if (top_ht) {
             GHashTableIter iter;
             const char *key;
@@ -106,8 +141,23 @@ static void qmp_input_pop(QmpInputVisitor *qiv, Error **errp)
             if (g_hash_table_iter_next(&iter, (void **)&key, NULL)) {
                 error_setg(errp, QERR_QMP_EXTRA_MEMBER, key);
             }
+        }
+    }
+}
+
+static void qmp_input_pop(Visitor *v)
+{
+    QmpInputVisitor *qiv = to_qiv(v);
+    StackObject *tos = &qiv->stack[qiv->nb_stack - 1];
+
+    assert(qiv->nb_stack > 0);
+
+    if (qiv->strict) {
+        GHashTable * const top_ht = qiv->stack[qiv->nb_stack - 1].h;
+        if (top_ht) {
             g_hash_table_unref(top_ht);
         }
+        tos->h = NULL;
     }
 
     qiv->nb_stack--;
@@ -120,6 +170,9 @@ static void qmp_input_start_struct(Visitor *v, const char *name, void **obj,
     QObject *qobj = qmp_input_get_object(qiv, name, true);
     Error *err = NULL;
 
+    if (obj) {
+        *obj = NULL;
+    }
     if (!qobj || qobject_type(qobj) != QTYPE_QDICT) {
         error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
                    "QDict");
@@ -137,63 +190,46 @@ static void qmp_input_start_struct(Visitor *v, const char *name, void **obj,
     }
 }
 
-static void qmp_input_end_struct(Visitor *v, Error **errp)
-{
-    QmpInputVisitor *qiv = to_qiv(v);
-
-    qmp_input_pop(qiv, errp);
-}
 
-static void qmp_input_start_list(Visitor *v, const char *name, Error **errp)
+static void qmp_input_start_list(Visitor *v, const char *name,
+                                 GenericList **list, size_t size, Error **errp)
 {
     QmpInputVisitor *qiv = to_qiv(v);
     QObject *qobj = qmp_input_get_object(qiv, name, true);
+    const QListEntry *entry;
 
     if (!qobj || qobject_type(qobj) != QTYPE_QLIST) {
+        if (list) {
+            *list = NULL;
+        }
         error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
                    "list");
         return;
     }
 
-    qmp_input_push(qiv, qobj, errp);
+    entry = qmp_input_push(qiv, qobj, errp);
+    if (list) {
+        if (entry) {
+            *list = g_malloc0(size);
+        } else {
+            *list = NULL;
+        }
+    }
 }
 
-static GenericList *qmp_input_next_list(Visitor *v, GenericList **list,
+static GenericList *qmp_input_next_list(Visitor *v, GenericList *tail,
                                         size_t size)
 {
     QmpInputVisitor *qiv = to_qiv(v);
-    GenericList *entry;
     StackObject *so = &qiv->stack[qiv->nb_stack - 1];
-    bool first;
 
-    if (so->entry == NULL) {
-        so->entry = qlist_first(qobject_to_qlist(so->obj));
-        first = true;
-    } else {
-        so->entry = qlist_next(so->entry);
-        first = false;
-    }
-
-    if (so->entry == NULL) {
+    if (!so->entry) {
         return NULL;
     }
-
-    entry = g_malloc0(size);
-    if (first) {
-        *list = entry;
-    } else {
-        (*list)->next = entry;
-    }
-
-    return entry;
+    tail->next = g_malloc0(size);
+    return tail->next;
 }
 
-static void qmp_input_end_list(Visitor *v)
-{
-    QmpInputVisitor *qiv = to_qiv(v);
-
-    qmp_input_pop(qiv, &error_abort);
-}
 
 static void qmp_input_start_alternate(Visitor *v, const char *name,
                                       GenericAlternate **obj, size_t size,
@@ -267,6 +303,7 @@ static void qmp_input_type_str(Visitor *v, const char *name, char **obj,
     QString *qstr = qobject_to_qstring(qmp_input_get_object(qiv, name, true));
 
     if (!qstr) {
+        *obj = NULL;
         error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
                    "string");
         return;
@@ -309,11 +346,22 @@ static void qmp_input_type_any(Visitor *v, const char *name, QObject **obj,
     *obj = qobj;
 }
 
-static void qmp_input_optional(Visitor *v, const char *name, bool *present)
+static void qmp_input_type_null(Visitor *v, const char *name, Error **errp)
 {
     QmpInputVisitor *qiv = to_qiv(v);
     QObject *qobj = qmp_input_get_object(qiv, name, true);
 
+    if (qobject_type(qobj) != QTYPE_QNULL) {
+        error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
+                   "null");
+    }
+}
+
+static void qmp_input_optional(Visitor *v, const char *name, bool *present)
+{
+    QmpInputVisitor *qiv = to_qiv(v);
+    QObject *qobj = qmp_input_get_object(qiv, name, false);
+
     if (!qobj) {
         *present = false;
         return;
@@ -329,43 +377,36 @@ Visitor *qmp_input_get_visitor(QmpInputVisitor *v)
 
 void qmp_input_visitor_cleanup(QmpInputVisitor *v)
 {
-    qobject_decref(v->stack[0].obj);
+    qobject_decref(v->root);
     g_free(v);
 }
 
-QmpInputVisitor *qmp_input_visitor_new(QObject *obj)
+QmpInputVisitor *qmp_input_visitor_new(QObject *obj, bool strict)
 {
     QmpInputVisitor *v;
 
     v = g_malloc0(sizeof(*v));
 
+    v->visitor.type = VISITOR_INPUT;
     v->visitor.start_struct = qmp_input_start_struct;
-    v->visitor.end_struct = qmp_input_end_struct;
+    v->visitor.check_struct = qmp_input_check_struct;
+    v->visitor.end_struct = qmp_input_pop;
     v->visitor.start_list = qmp_input_start_list;
     v->visitor.next_list = qmp_input_next_list;
-    v->visitor.end_list = qmp_input_end_list;
+    v->visitor.end_list = qmp_input_pop;
     v->visitor.start_alternate = qmp_input_start_alternate;
-    v->visitor.type_enum = input_type_enum;
     v->visitor.type_int64 = qmp_input_type_int64;
     v->visitor.type_uint64 = qmp_input_type_uint64;
     v->visitor.type_bool = qmp_input_type_bool;
     v->visitor.type_str = qmp_input_type_str;
     v->visitor.type_number = qmp_input_type_number;
     v->visitor.type_any = qmp_input_type_any;
+    v->visitor.type_null = qmp_input_type_null;
     v->visitor.optional = qmp_input_optional;
+    v->strict = strict;
 
-    qmp_input_push(v, obj, NULL);
+    v->root = obj;
     qobject_incref(obj);
 
     return v;
 }
-
-QmpInputVisitor *qmp_input_visitor_new_strict(QObject *obj)
-{
-    QmpInputVisitor *v;
-
-    v = qmp_input_visitor_new(obj);
-    v->strict = true;
-
-    return v;
-}

qapi/qmp-output-visitor.c

@@ -22,7 +22,6 @@
 typedef struct QStackEntry
 {
     QObject *value;
-    bool is_list_head;
     QTAILQ_ENTRY(QStackEntry) node;
 } QStackEntry;
 
@@ -52,9 +51,6 @@ static void qmp_output_push_obj(QmpOutputVisitor *qov, QObject *value)
     assert(qov->root);
     assert(value);
     e->value = value;
-    if (qobject_type(e->value) == QTYPE_QLIST) {
-        e->is_list_head = true;
-    }
     QTAILQ_INSERT_HEAD(&qov->stack, e, node);
 }
 
@@ -82,9 +78,8 @@ static void qmp_output_add_obj(QmpOutputVisitor *qov, const char *name,
     QObject *cur = e ? e->value : NULL;
 
     if (!cur) {
-        /* FIXME we should require the user to reset the visitor, rather
-         * than throwing away the previous root */
-        qobject_decref(qov->root);
+        /* Don't allow reuse of visitor on more than one root */
+        assert(!qov->root);
         qov->root = value;
     } else {
         switch (qobject_type(cur)) {
@@ -93,6 +88,7 @@ static void qmp_output_add_obj(QmpOutputVisitor *qov, const char *name,
             qdict_put_obj(qobject_to_qdict(cur), name, value);
             break;
         case QTYPE_QLIST:
+            assert(!name);
             qlist_append_obj(qobject_to_qlist(cur), value);
             break;
         default:
@@ -111,13 +107,16 @@ static void qmp_output_start_struct(Visitor *v, const char *name, void **obj,
     qmp_output_push(qov, dict);
 }
 
-static void qmp_output_end_struct(Visitor *v, Error **errp)
+static void qmp_output_end_struct(Visitor *v)
 {
     QmpOutputVisitor *qov = to_qov(v);
-    qmp_output_pop(qov);
+    QObject *value = qmp_output_pop(qov);
+    assert(qobject_type(value) == QTYPE_QDICT);
 }
 
-static void qmp_output_start_list(Visitor *v, const char *name, Error **errp)
+static void qmp_output_start_list(Visitor *v, const char *name,
+                                  GenericList **listp, size_t size,
+                                  Error **errp)
 {
     QmpOutputVisitor *qov = to_qov(v);
     QList *list = qlist_new();
@@ -126,26 +125,17 @@ static void qmp_output_start_list(Visitor *v, const char *name, Error **errp)
     qmp_output_push(qov, list);
 }
 
-static GenericList *qmp_output_next_list(Visitor *v, GenericList **listp,
+static GenericList *qmp_output_next_list(Visitor *v, GenericList *tail,
                                          size_t size)
 {
-    GenericList *list = *listp;
-    QmpOutputVisitor *qov = to_qov(v);
-    QStackEntry *e = QTAILQ_FIRST(&qov->stack);
-
-    assert(e);
-    if (e->is_list_head) {
-        e->is_list_head = false;
-        return list;
-    }
-
-    return list ? list->next : NULL;
+    return tail->next;
 }
 
 static void qmp_output_end_list(Visitor *v)
 {
     QmpOutputVisitor *qov = to_qov(v);
-    qmp_output_pop(qov);
+    QObject *value = qmp_output_pop(qov);
+    assert(qobject_type(value) == QTYPE_QLIST);
 }
 
 static void qmp_output_type_int64(Visitor *v, const char *name, int64_t *obj,
@@ -196,18 +186,22 @@ static void qmp_output_type_any(Visitor *v, const char *name, QObject **obj,
     qmp_output_add_obj(qov, name, *obj);
 }
 
-/* Finish building, and return the root object. Will not be NULL. */
+static void qmp_output_type_null(Visitor *v, const char *name, Error **errp)
+{
+    QmpOutputVisitor *qov = to_qov(v);
+    qmp_output_add_obj(qov, name, qnull());
+}
+
+/* Finish building, and return the root object.
+ * The root object is never null. The caller becomes the object's
+ * owner, and should use qobject_decref() when done with it.  */
 QObject *qmp_output_get_qobject(QmpOutputVisitor *qov)
 {
-    /* FIXME: we should require that a visit occurred, and that it is
-     * complete (no starts without a matching end) */
-    QObject *obj = qov->root;
-    if (obj) {
-        qobject_incref(obj);
-    } else {
-        obj = qnull();
-    }
-    return obj;
+    /* A visit must have occurred, with each start paired with end.  */
+    assert(qov->root && QTAILQ_EMPTY(&qov->stack));
+
+    qobject_incref(qov->root);
+    return qov->root;
 }
 
 Visitor *qmp_output_get_visitor(QmpOutputVisitor *v)
@@ -234,18 +228,19 @@ QmpOutputVisitor *qmp_output_visitor_new(void)
 
     v = g_malloc0(sizeof(*v));
 
+    v->visitor.type = VISITOR_OUTPUT;
     v->visitor.start_struct = qmp_output_start_struct;
     v->visitor.end_struct = qmp_output_end_struct;
     v->visitor.start_list = qmp_output_start_list;
     v->visitor.next_list = qmp_output_next_list;
     v->visitor.end_list = qmp_output_end_list;
-    v->visitor.type_enum = output_type_enum;
     v->visitor.type_int64 = qmp_output_type_int64;
     v->visitor.type_uint64 = qmp_output_type_uint64;
     v->visitor.type_bool = qmp_output_type_bool;
     v->visitor.type_str = qmp_output_type_str;
     v->visitor.type_number = qmp_output_type_number;
     v->visitor.type_any = qmp_output_type_any;
+    v->visitor.type_null = qmp_output_type_null;
 
     QTAILQ_INIT(&v->stack);
 

qapi/qmp-registry.c

@@ -25,7 +25,6 @@ void qmp_register_command(const char *name, QmpCommandFunc *fn,
     QmpCommand *cmd = g_malloc0(sizeof(*cmd));
 
     cmd->name = name;
-    cmd->type = QCT_NORMAL;
     cmd->fn = fn;
     cmd->enabled = true;
     cmd->options = options;

qapi/string-input-visitor.c

@@ -25,8 +25,6 @@ struct StringInputVisitor
 {
     Visitor visitor;
 
-    bool head;
-
     GList *ranges;
     GList *cur_range;
     int64_t cur;
@@ -44,7 +42,7 @@ static void free_range(void *range, void *dummy)
     g_free(range);
 }
 
-static void parse_str(StringInputVisitor *siv, Error **errp)
+static int parse_str(StringInputVisitor *siv, const char *name, Error **errp)
 {
     char *str = (char *) siv->string;
     long long start, end;
@@ -52,7 +50,7 @@ static void parse_str(StringInputVisitor *siv, Error **errp)
     char *endptr;
 
     if (siv->ranges) {
-        return;
+        return 0;
     }
 
     do {
@@ -117,19 +115,29 @@ static void parse_str(StringInputVisitor *siv, Error **errp)
         }
     } while (str);
 
-    return;
+    return 0;
 error:
     g_list_foreach(siv->ranges, free_range, NULL);
     g_list_free(siv->ranges);
     siv->ranges = NULL;
+    error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
+               "an int64 value or range");
+    return -1;
 }
 
 static void
-start_list(Visitor *v, const char *name, Error **errp)
+start_list(Visitor *v, const char *name, GenericList **list, size_t size,
+           Error **errp)
 {
     StringInputVisitor *siv = to_siv(v);
 
-    parse_str(siv, errp);
+    /* We don't support visits without a list */
+    assert(list);
+
+    if (parse_str(siv, name, errp) < 0) {
+        *list = NULL;
+        return;
+    }
 
     siv->cur_range = g_list_first(siv->ranges);
     if (siv->cur_range) {
@@ -137,13 +145,15 @@ start_list(Visitor *v, const char *name, Error **errp)
         if (r) {
             siv->cur = r->begin;
         }
+        *list = g_malloc0(size);
+    } else {
+        *list = NULL;
     }
 }
 
-static GenericList *next_list(Visitor *v, GenericList **list, size_t size)
+static GenericList *next_list(Visitor *v, GenericList *tail, size_t size)
 {
     StringInputVisitor *siv = to_siv(v);
-    GenericList **link;
     Range *r;
 
     if (!siv->ranges || !siv->cur_range) {
@@ -167,21 +177,12 @@ static GenericList *next_list(Visitor *v, GenericList **list, size_t size)
         siv->cur = r->begin;
     }
 
-    if (siv->head) {
-        link = list;
-        siv->head = false;
-    } else {
-        link = &(*list)->next;
-    }
-
-    *link = g_malloc0(size);
-    return *link;
+    tail->next = g_malloc0(size);
+    return tail->next;
 }
 
 static void end_list(Visitor *v)
 {
-    StringInputVisitor *siv = to_siv(v);
-    siv->head = true;
 }
 
 static void parse_type_int64(Visitor *v, const char *name, int64_t *obj,
@@ -195,7 +196,9 @@ static void parse_type_int64(Visitor *v, const char *name, int64_t *obj,
         return;
     }
 
-    parse_str(siv, errp);
+    if (parse_str(siv, name, errp) < 0) {
+        return;
+    }
 
     if (!siv->ranges) {
         goto error;
@@ -293,6 +296,7 @@ static void parse_type_str(Visitor *v, const char *name, char **obj,
     if (siv->string) {
         *obj = g_strdup(siv->string);
     } else {
+        *obj = NULL;
         error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
                    "string");
     }
@@ -348,7 +352,7 @@ StringInputVisitor *string_input_visitor_new(const char *str)
 
     v = g_malloc0(sizeof(*v));
 
-    v->visitor.type_enum = input_type_enum;
+    v->visitor.type = VISITOR_INPUT;
     v->visitor.type_int64 = parse_type_int64;
     v->visitor.type_uint64 = parse_type_uint64;
     v->visitor.type_size = parse_type_size;
@@ -361,6 +365,5 @@ StringInputVisitor *string_input_visitor_new(const char *str)
     v->visitor.optional = parse_optional;
 
     v->string = str;
-    v->head = true;
     return v;
 }

qapi/string-output-visitor.c

@@ -20,7 +20,7 @@
 
 enum ListMode {
     LM_NONE,             /* not traversing a list of repeated options */
-    LM_STARTED,          /* start_list() succeeded */
+    LM_STARTED,          /* next_list() ready to be called */
 
     LM_IN_PROGRESS,      /* next_list() has been called.
                           *
@@ -48,7 +48,7 @@ enum ListMode {
 
     LM_UNSIGNED_INTERVAL,/* Same as above, only for an unsigned interval. */
 
-    LM_END
+    LM_END,              /* next_list() called, about to see last element. */
 };
 
 typedef enum ListMode ListMode;
@@ -58,7 +58,6 @@ struct StringOutputVisitor
     Visitor visitor;
     bool human;
     GString *string;
-    bool head;
     ListMode list_mode;
     union {
         int64_t s;
@@ -266,39 +265,29 @@ static void print_type_number(Visitor *v, const char *name, double *obj,
 }
 
 static void
-start_list(Visitor *v, const char *name, Error **errp)
+start_list(Visitor *v, const char *name, GenericList **list, size_t size,
+           Error **errp)
 {
     StringOutputVisitor *sov = to_sov(v);
 
     /* we can't traverse a list in a list */
     assert(sov->list_mode == LM_NONE);
-    sov->list_mode = LM_STARTED;
-    sov->head = true;
+    /* We don't support visits without a list */
+    assert(list);
+    /* List handling is only needed if there are at least two elements */
+    if (*list && (*list)->next) {
+        sov->list_mode = LM_STARTED;
+    }
 }
 
-static GenericList *next_list(Visitor *v, GenericList **list, size_t size)
+static GenericList *next_list(Visitor *v, GenericList *tail, size_t size)
 {
     StringOutputVisitor *sov = to_sov(v);
-    GenericList *ret = NULL;
-    if (*list) {
-        if (sov->head) {
-            ret = *list;
-        } else {
-            ret = (*list)->next;
-        }
+    GenericList *ret = tail->next;
 
-        if (sov->head) {
-            if (ret && ret->next == NULL) {
-                sov->list_mode = LM_NONE;
-            }
-            sov->head = false;
-        } else {
-            if (ret && ret->next == NULL) {
-                sov->list_mode = LM_END;
-            }
-        }
+    if (ret && !ret->next) {
+        sov->list_mode = LM_END;
     }
-
     return ret;
 }
 
@@ -311,8 +300,6 @@ static void end_list(Visitor *v)
            sov->list_mode == LM_NONE ||
            sov->list_mode == LM_IN_PROGRESS);
     sov->list_mode = LM_NONE;
-    sov->head = true;
-
 }
 
 char *string_output_get_string(StringOutputVisitor *sov)
@@ -351,7 +338,7 @@ StringOutputVisitor *string_output_visitor_new(bool human)
 
     v->string = g_string_new(NULL);
     v->human = human;
-    v->visitor.type_enum = output_type_enum;
+    v->visitor.type = VISITOR_OUTPUT;
     v->visitor.type_int64 = print_type_int64;
     v->visitor.type_uint64 = print_type_uint64;
     v->visitor.type_size = print_type_size;

qmp.c

@@ -663,7 +663,7 @@ void qmp_object_add(const char *type, const char *id,
         }
     }
 
-    qiv = qmp_input_visitor_new(props);
+    qiv = qmp_input_visitor_new(props, true);
     obj = user_creatable_add_type(type, id, pdict,
                                   qmp_input_get_visitor(qiv), errp);
     qmp_input_visitor_cleanup(qiv);

qom/object.c

@@ -2036,10 +2036,9 @@ static void property_get_tm(Object *obj, Visitor *v, const char *name,
     if (err) {
         goto out_end;
     }
+    visit_check_struct(v, &err);
 out_end:
-    error_propagate(errp, err);
-    err = NULL;
-    visit_end_struct(v, errp);
+    visit_end_struct(v);
 out:
     error_propagate(errp, err);
 

qom/object_interfaces.c

@@ -42,7 +42,7 @@ Object *user_creatable_add(const QDict *qdict,
     char *type = NULL;
     char *id = NULL;
     Object *obj = NULL;
-    Error *local_err = NULL, *end_err = NULL;
+    Error *local_err = NULL;
     QDict *pdict;
 
     pdict = qdict_clone_shallow(qdict);
@@ -63,21 +63,15 @@ Object *user_creatable_add(const QDict *qdict,
     if (local_err) {
         goto out_visit;
     }
-
-    obj = user_creatable_add_type(type, id, pdict, v, &local_err);
+    visit_check_struct(v, &local_err);
     if (local_err) {
         goto out_visit;
     }
 
- out_visit:
-    visit_end_struct(v, &end_err);
-    if (end_err) {
-        error_propagate(&local_err, end_err);
-        if (obj) {
-            user_creatable_del(id, NULL);
-        }
-        goto out;
-    }
+    obj = user_creatable_add_type(type, id, pdict, v, &local_err);
+
+out_visit:
+    visit_end_struct(v);
 
 out:
     QDECREF(pdict);
@@ -118,15 +112,25 @@ Object *user_creatable_add_type(const char *type, const char *id,
         return NULL;
     }
 
+    assert(qdict);
     obj = object_new(type);
-    if (qdict) {
-        for (e = qdict_first(qdict); e; e = qdict_next(qdict, e)) {
-            object_property_set(obj, v, e->key, &local_err);
-            if (local_err) {
-                goto out;
-            }
+    visit_start_struct(v, NULL, NULL, 0, &local_err);
+    if (local_err) {
+        goto out;
+    }
+    for (e = qdict_first(qdict); e; e = qdict_next(qdict, e)) {
+        object_property_set(obj, v, e->key, &local_err);
+        if (local_err) {
+            break;
         }
     }
+    if (!local_err) {
+        visit_check_struct(v, &local_err);
+    }
+    visit_end_struct(v);
+    if (local_err) {
+        goto out;
+    }
 
     object_property_add_child(object_get_objects_root(),
                               id, obj, &local_err);

qom/qom-qobject.c

@@ -22,7 +22,8 @@ void object_property_set_qobject(Object *obj, QObject *value,
                                  const char *name, Error **errp)
 {
     QmpInputVisitor *qiv;
-    qiv = qmp_input_visitor_new(value);
+    /* TODO: Should we reject, rather than ignore, excess input? */
+    qiv = qmp_input_visitor_new(value, false);
     object_property_set(obj, qmp_input_get_visitor(qiv), name, errp);
 
     qmp_input_visitor_cleanup(qiv);

replay/replay-input.c

@@ -37,7 +37,7 @@ static InputEvent *qapi_clone_InputEvent(InputEvent *src)
         return NULL;
     }
 
-    qiv = qmp_input_visitor_new(obj);
+    qiv = qmp_input_visitor_new(obj, true);
     iv = qmp_input_get_visitor(qiv);
     visit_type_InputEvent(iv, NULL, &dst, &error_abort);
     qmp_input_visitor_cleanup(qiv);

scripts/qapi-commands.py

@@ -115,13 +115,21 @@ def gen_marshal(name, arg_type, ret_type):
 
     if arg_type and arg_type.members:
         ret += mcgen('''
-    QmpInputVisitor *qiv = qmp_input_visitor_new_strict(QOBJECT(args));
+    QmpInputVisitor *qiv = qmp_input_visitor_new(QOBJECT(args), true);
     QapiDeallocVisitor *qdv;
     Visitor *v;
     %(c_name)s arg = {0};
 
     v = qmp_input_get_visitor(qiv);
+    visit_start_struct(v, NULL, NULL, 0, &err);
+    if (err) {
+        goto out;
+    }
     visit_type_%(c_name)s_members(v, &arg, &err);
+    if (!err) {
+        visit_check_struct(v, &err);
+    }
+    visit_end_struct(v);
     if (err) {
         goto out;
     }
@@ -150,7 +158,9 @@ out:
     qmp_input_visitor_cleanup(qiv);
     qdv = qapi_dealloc_visitor_new();
     v = qapi_dealloc_get_visitor(qdv);
+    visit_start_struct(v, NULL, NULL, 0, NULL);
     visit_type_%(c_name)s_members(v, &arg, NULL);
+    visit_end_struct(v);
     qapi_dealloc_visitor_cleanup(qdv);
 ''',
                      c_name=arg_type.c_name())

scripts/qapi-event.py

@@ -98,7 +98,10 @@ def gen_event_send(name, arg_type):
         goto out;
     }
     visit_type_%(c_name)s_members(v, &param, &err);
-    visit_end_struct(v, err ? NULL : &err);
+    if (!err) {
+        visit_check_struct(v, &err);
+    }
+    visit_end_struct(v);
     if (err) {
         goto out;
     }

scripts/qapi-visit.py

@@ -108,30 +108,32 @@ out:
 
 
 def gen_visit_list(name, element_type):
-    # FIXME: if *obj is NULL on entry, and the first visit_next_list()
-    # assigns to *obj, while a later one fails, we should clean up *obj
-    # rather than leaving it non-NULL. As currently written, the caller must
-    # call qapi_free_FOOList() to avoid a memory leak of the partial FOOList.
     return mcgen('''
 
 void visit_type_%(c_name)s(Visitor *v, const char *name, %(c_name)s **obj, Error **errp)
 {
     Error *err = NULL;
-    GenericList *i, **prev;
+    %(c_name)s *tail;
+    size_t size = sizeof(**obj);
 
-    visit_start_list(v, name, &err);
+    visit_start_list(v, name, (GenericList **)obj, size, &err);
     if (err) {
         goto out;
     }
 
-    for (prev = (GenericList **)obj;
-         !err && (i = visit_next_list(v, prev, sizeof(**obj))) != NULL;
-         prev = &i) {
-        %(c_name)s *native_i = (%(c_name)s *)i;
-        visit_type_%(c_elt_type)s(v, NULL, &native_i->value, &err);
+    for (tail = *obj; tail;
+         tail = (%(c_name)s *)visit_next_list(v, (GenericList *)tail, size)) {
+        visit_type_%(c_elt_type)s(v, NULL, &tail->value, &err);
+        if (err) {
+            break;
+        }
     }
 
     visit_end_list(v);
+    if (err && visit_is_input(v)) {
+        qapi_free_%(c_name)s(*obj);
+        *obj = NULL;
+    }
 out:
     error_propagate(errp, err);
 }
@@ -186,9 +188,10 @@ void visit_type_%(c_name)s(Visitor *v, const char *name, %(c_name)s **obj, Error
             break;
         }
         visit_type_%(c_type)s_members(v, &(*obj)->u.%(c_name)s, &err);
-        error_propagate(errp, err);
-        err = NULL;
-        visit_end_struct(v, &err);
+        if (!err) {
+            visit_check_struct(v, &err);
+        }
+        visit_end_struct(v);
 ''',
                          c_type=var.type.c_name(),
                          c_name=c_name(var.name))
@@ -208,20 +211,20 @@ void visit_type_%(c_name)s(Visitor *v, const char *name, %(c_name)s **obj, Error
                    "%(name)s");
     }
     visit_end_alternate(v);
+    if (err && visit_is_input(v)) {
+        qapi_free_%(c_name)s(*obj);
+        *obj = NULL;
+    }
 out:
     error_propagate(errp, err);
 }
 ''',
-                 name=name)
+                 name=name, c_name=c_name(name))
 
     return ret
 
 
 def gen_visit_object(name, base, members, variants):
-    # FIXME: if *obj is NULL on entry, and visit_start_struct() assigns to
-    # *obj, but then visit_type_FOO_members() fails, we should clean up *obj
-    # rather than leaving it non-NULL. As currently written, the caller must
-    # call qapi_free_FOO() to avoid a memory leak of the partial FOO.
     return mcgen('''
 
 void visit_type_%(c_name)s(Visitor *v, const char *name, %(c_name)s **obj, Error **errp)
@@ -236,10 +239,16 @@ void visit_type_%(c_name)s(Visitor *v, const char *name, %(c_name)s **obj, Error
         goto out_obj;
     }
     visit_type_%(c_name)s_members(v, *obj, &err);
-    error_propagate(errp, err);
-    err = NULL;
+    if (err) {
+        goto out_obj;
+    }
+    visit_check_struct(v, &err);
 out_obj:
-    visit_end_struct(v, &err);
+    visit_end_struct(v);
+    if (err && visit_is_input(v)) {
+        qapi_free_%(c_name)s(*obj);
+        *obj = NULL;
+    }
 out:
     error_propagate(errp, err);
 }

tests/.gitignore

@@ -3,6 +3,7 @@ check-qfloat
 check-qint
 check-qjson
 check-qlist
+check-qnull
 check-qstring
 check-qom-interface
 check-qom-proplist

tests/Makefile

@@ -16,6 +16,8 @@ check-unit-y += tests/check-qstring$(EXESUF)
 gcov-files-check-qstring-y = qobject/qstring.c
 check-unit-y += tests/check-qlist$(EXESUF)
 gcov-files-check-qlist-y = qobject/qlist.c
+check-unit-y += tests/check-qnull$(EXESUF)
+gcov-files-check-qnull-y = qobject/qnull.c
 check-unit-y += tests/check-qjson$(EXESUF)
 gcov-files-check-qjson-y = qobject/qjson.c
 check-unit-y += tests/test-qmp-output-visitor$(EXESUF)
@@ -382,7 +384,8 @@ GENERATED_HEADERS += tests/test-qapi-types.h tests/test-qapi-visit.h \
 	tests/test-qmp-introspect.h
 
 test-obj-y = tests/check-qint.o tests/check-qstring.o tests/check-qdict.o \
-	tests/check-qlist.o tests/check-qfloat.o tests/check-qjson.o \
+	tests/check-qlist.o tests/check-qfloat.o tests/check-qnull.o \
+	tests/check-qjson.o \
 	tests/test-coroutine.o tests/test-string-output-visitor.o \
 	tests/test-string-input-visitor.o tests/test-qmp-output-visitor.o \
 	tests/test-qmp-input-visitor.o tests/test-qmp-input-strict.o \
@@ -410,6 +413,7 @@ tests/check-qstring$(EXESUF): tests/check-qstring.o $(test-util-obj-y)
 tests/check-qdict$(EXESUF): tests/check-qdict.o $(test-util-obj-y)
 tests/check-qlist$(EXESUF): tests/check-qlist.o $(test-util-obj-y)
 tests/check-qfloat$(EXESUF): tests/check-qfloat.o $(test-util-obj-y)
+tests/check-qnull$(EXESUF): tests/check-qnull.o $(test-util-obj-y)
 tests/check-qjson$(EXESUF): tests/check-qjson.o $(test-util-obj-y)
 tests/check-qom-interface$(EXESUF): tests/check-qom-interface.o $(test-qom-obj-y)
 tests/check-qom-proplist$(EXESUF): tests/check-qom-proplist.o $(test-qom-obj-y)

tests/check-qnull.c

+/*
+ * QNull unit-tests.
+ *
+ * Copyright (C) 2016 Red Hat Inc.
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ */
+#include "qemu/osdep.h"
+#include <glib.h>
+
+#include "qapi/qmp/qobject.h"
+#include "qemu-common.h"
+#include "qapi/qmp-input-visitor.h"
+#include "qapi/qmp-output-visitor.h"
+#include "qapi/error.h"
+
+/*
+ * Public Interface test-cases
+ *
+ * (with some violations to access 'private' data)
+ */
+
+static void qnull_ref_test(void)
+{
+    QObject *obj;
+
+    g_assert(qnull_.refcnt == 1);
+    obj = qnull();
+    g_assert(obj);
+    g_assert(obj == &qnull_);
+    g_assert(qnull_.refcnt == 2);
+    g_assert(qobject_type(obj) == QTYPE_QNULL);
+    qobject_decref(obj);
+    g_assert(qnull_.refcnt == 1);
+}
+
+static void qnull_visit_test(void)
+{
+    QObject *obj;
+    QmpOutputVisitor *qov;
+    QmpInputVisitor *qiv;
+
+    /*
+     * Most tests of interactions between QObject and visitors are in
+     * test-qmp-*-visitor; but these tests live here because they
+     * depend on layering violations to check qnull_ refcnt.
+     */
+
+    g_assert(qnull_.refcnt == 1);
+    obj = qnull();
+    qiv = qmp_input_visitor_new(obj, true);
+    qobject_decref(obj);
+    visit_type_null(qmp_input_get_visitor(qiv), NULL, &error_abort);
+    qmp_input_visitor_cleanup(qiv);
+
+    qov = qmp_output_visitor_new();
+    visit_type_null(qmp_output_get_visitor(qov), NULL, &error_abort);
+    obj = qmp_output_get_qobject(qov);
+    g_assert(obj == &qnull_);
+    qobject_decref(obj);
+    qmp_output_visitor_cleanup(qov);
+
+    g_assert(qnull_.refcnt == 1);
+}
+
+int main(int argc, char **argv)
+{
+    g_test_init(&argc, &argv, NULL);
+
+    g_test_add_func("/public/qnull_ref", qnull_ref_test);
+    g_test_add_func("/public/qnull_visit", qnull_visit_test);
+
+    return g_test_run();
+}

tests/test-qmp-commands.c

@@ -222,20 +222,19 @@ static void test_dealloc_partial(void)
         ud2_dict = qdict_new();
         qdict_put_obj(ud2_dict, "string0", QOBJECT(qstring_from_str(text)));
 
-        qiv = qmp_input_visitor_new(QOBJECT(ud2_dict));
+        qiv = qmp_input_visitor_new(QOBJECT(ud2_dict), true);
         visit_type_UserDefTwo(qmp_input_get_visitor(qiv), NULL, &ud2, &err);
         qmp_input_visitor_cleanup(qiv);
         QDECREF(ud2_dict);
     }
 
-    /* verify partial success */
-    assert(ud2 != NULL);
-    assert(ud2->string0 != NULL);
-    assert(strcmp(ud2->string0, text) == 0);
-    assert(ud2->dict1 == NULL);
-
-    /* confirm & release construction error */
+    /* verify that visit_type_XXX() cleans up properly on error */
     error_free_or_abort(&err);
+    assert(!ud2);
+
+    /* Manually create a partial object, leaving ud2->dict1 at NULL */
+    ud2 = g_new0(UserDefTwo, 1);
+    ud2->string0 = g_strdup(text);
 
     /* tear down partial object */
     qapi_free_UserDefTwo(ud2);

tests/test-qmp-input-strict.c

@@ -55,7 +55,7 @@ static Visitor *validate_test_init_internal(TestInputVisitorData *data,
     data->obj = qobject_from_jsonv(json_string, ap);
     g_assert(data->obj);
 
-    data->qiv = qmp_input_visitor_new_strict(data->obj);
+    data->qiv = qmp_input_visitor_new(data->obj, true);
     g_assert(data->qiv);
 
     v = qmp_input_get_visitor(data->qiv);
@@ -182,10 +182,7 @@ static void test_validate_fail_struct(TestInputVisitorData *data,
 
     visit_type_TestStruct(v, NULL, &p, &err);
     error_free_or_abort(&err);
-    if (p) {
-        g_free(p->string);
-    }
-    g_free(p);
+    g_assert(!p);
 }
 
 static void test_validate_fail_struct_nested(TestInputVisitorData *data,
@@ -199,7 +196,7 @@ static void test_validate_fail_struct_nested(TestInputVisitorData *data,
 
     visit_type_UserDefTwo(v, NULL, &udp, &err);
     error_free_or_abort(&err);
-    qapi_free_UserDefTwo(udp);
+    g_assert(!udp);
 }
 
 static void test_validate_fail_list(TestInputVisitorData *data,
@@ -213,7 +210,7 @@ static void test_validate_fail_list(TestInputVisitorData *data,
 
     visit_type_UserDefOneList(v, NULL, &head, &err);
     error_free_or_abort(&err);
-    qapi_free_UserDefOneList(head);
+    g_assert(!head);
 }
 
 static void test_validate_fail_union_native_list(TestInputVisitorData *data,
@@ -228,7 +225,7 @@ static void test_validate_fail_union_native_list(TestInputVisitorData *data,
 
     visit_type_UserDefNativeListUnion(v, NULL, &tmp, &err);
     error_free_or_abort(&err);
-    qapi_free_UserDefNativeListUnion(tmp);
+    g_assert(!tmp);
 }
 
 static void test_validate_fail_union_flat(TestInputVisitorData *data,
@@ -242,7 +239,7 @@ static void test_validate_fail_union_flat(TestInputVisitorData *data,
 
     visit_type_UserDefFlatUnion(v, NULL, &tmp, &err);
     error_free_or_abort(&err);
-    qapi_free_UserDefFlatUnion(tmp);
+    g_assert(!tmp);
 }
 
 static void test_validate_fail_union_flat_no_discrim(TestInputVisitorData *data,
@@ -257,13 +254,13 @@ static void test_validate_fail_union_flat_no_discrim(TestInputVisitorData *data,
 
     visit_type_UserDefFlatUnion2(v, NULL, &tmp, &err);
     error_free_or_abort(&err);
-    qapi_free_UserDefFlatUnion2(tmp);
+    g_assert(!tmp);
 }
 
 static void test_validate_fail_alternate(TestInputVisitorData *data,
                                          const void *unused)
 {
-    UserDefAlternate *tmp = NULL;
+    UserDefAlternate *tmp;
     Visitor *v;
     Error *err = NULL;
 
@@ -271,7 +268,7 @@ static void test_validate_fail_alternate(TestInputVisitorData *data,
 
     visit_type_UserDefAlternate(v, NULL, &tmp, &err);
     error_free_or_abort(&err);
-    qapi_free_UserDefAlternate(tmp);
+    g_assert(!tmp);
 }
 
 static void do_test_validate_qmp_introspect(TestInputVisitorData *data,

tests/test-qmp-input-visitor.c

@@ -51,7 +51,7 @@ static Visitor *visitor_input_test_init_internal(TestInputVisitorData *data,
     data->obj = qobject_from_jsonv(json_string, ap);
     g_assert(data->obj);
 
-    data->qiv = qmp_input_visitor_new(data->obj);
+    data->qiv = qmp_input_visitor_new(data->obj, false);
     g_assert(data->qiv);
 
     v = qmp_input_get_visitor(data->qiv);
@@ -279,6 +279,34 @@ static void test_visitor_in_any(TestInputVisitorData *data,
     qobject_decref(res);
 }
 
+static void test_visitor_in_null(TestInputVisitorData *data,
+                                 const void *unused)
+{
+    Visitor *v;
+    Error *err = NULL;
+    char *tmp;
+
+    /*
+     * FIXME: Since QAPI doesn't know the 'null' type yet, we can't
+     * test visit_type_null() by reading into a QAPI struct then
+     * checking that it was populated correctly.  The best we can do
+     * for now is ensure that we consumed null from the input, proven
+     * by the fact that we can't re-read the key; and that we detect
+     * when input is not null.
+     */
+
+    v = visitor_input_test_init(data, "{ 'a': null, 'b': '' }");
+    visit_start_struct(v, NULL, NULL, 0, &error_abort);
+    visit_type_null(v, "a", &error_abort);
+    visit_type_str(v, "a", &tmp, &err);
+    g_assert(!tmp);
+    error_free_or_abort(&err);
+    visit_type_null(v, "b", &err);
+    error_free_or_abort(&err);
+    visit_check_struct(v, &error_abort);
+    visit_end_struct(v);
+}
+
 static void test_visitor_in_union_flat(TestInputVisitorData *data,
                                        const void *unused)
 {
@@ -745,18 +773,12 @@ static void test_visitor_in_errors(TestInputVisitorData *data,
 
     visit_type_TestStruct(v, NULL, &p, &err);
     error_free_or_abort(&err);
-    /* FIXME - a failed parse should not leave a partially-allocated p
-     * for us to clean up; this could cause callers to leak memory. */
-    g_assert(p->string == NULL);
-
-    g_free(p->string);
-    g_free(p);
+    g_assert(!p);
 
     v = visitor_input_test_init(data, "[ '1', '2', false, '3' ]");
     visit_type_strList(v, NULL, &q, &err);
     error_free_or_abort(&err);
-    assert(q);
-    qapi_free_strList(q);
+    assert(!q);
 }
 
 static void test_visitor_in_wrong_type(TestInputVisitorData *data,
@@ -829,6 +851,8 @@ int main(int argc, char **argv)
                            &in_visitor_data, test_visitor_in_list);
     input_visitor_test_add("/visitor/input/any",
                            &in_visitor_data, test_visitor_in_any);
+    input_visitor_test_add("/visitor/input/null",
+                           &in_visitor_data, test_visitor_in_null);
     input_visitor_test_add("/visitor/input/union-flat",
                            &in_visitor_data, test_visitor_in_union_flat);
     input_visitor_test_add("/visitor/input/alternate",

tests/test-qmp-output-visitor.c

@@ -43,6 +43,12 @@ static void visitor_output_teardown(TestOutputVisitorData *data,
     data->ov = NULL;
 }
 
+static void visitor_reset(TestOutputVisitorData *data)
+{
+    visitor_output_teardown(data, NULL);
+    visitor_output_setup(data, NULL);
+}
+
 static void test_visitor_out_int(TestOutputVisitorData *data,
                                  const void *unused)
 {
@@ -139,6 +145,7 @@ static void test_visitor_out_enum(TestOutputVisitorData *data,
         g_assert_cmpstr(qstring_get_str(qobject_to_qstring(obj)), ==,
                         EnumOne_lookup[i]);
         qobject_decref(obj);
+        visitor_reset(data);
     }
 }
 
@@ -153,6 +160,7 @@ static void test_visitor_out_enum_errors(TestOutputVisitorData *data,
         visit_type_EnumOne(data->ov, "unused", &bad_values[i], &err);
         g_assert(err);
         error_free(err);
+        visitor_reset(data);
     }
 }
 
@@ -262,6 +270,7 @@ static void test_visitor_out_struct_errors(TestOutputVisitorData *data,
         visit_type_UserDefOne(data->ov, "unused", &pu, &err);
         g_assert(err);
         error_free(err);
+        visitor_reset(data);
     }
 }
 
@@ -366,6 +375,7 @@ static void test_visitor_out_any(TestOutputVisitorData *data,
     qobject_decref(obj);
     qobject_decref(qobj);
 
+    visitor_reset(data);
     qdict = qdict_new();
     qdict_put(qdict, "integer", qint_from_int(-42));
     qdict_put(qdict, "boolean", qbool_from_bool(true));
@@ -442,6 +452,7 @@ static void test_visitor_out_alternate(TestOutputVisitorData *data,
     qapi_free_UserDefAlternate(tmp);
     qobject_decref(arg);
 
+    visitor_reset(data);
     tmp = g_new0(UserDefAlternate, 1);
     tmp->type = QTYPE_QSTRING;
     tmp->u.s = g_strdup("hello");
@@ -455,6 +466,7 @@ static void test_visitor_out_alternate(TestOutputVisitorData *data,
     qapi_free_UserDefAlternate(tmp);
     qobject_decref(arg);
 
+    visitor_reset(data);
     tmp = g_new0(UserDefAlternate, 1);
     tmp->type = QTYPE_QDICT;
     tmp->u.udfu.integer = 1;
@@ -477,15 +489,24 @@ static void test_visitor_out_alternate(TestOutputVisitorData *data,
     qobject_decref(arg);
 }
 
-static void test_visitor_out_empty(TestOutputVisitorData *data,
-                                   const void *unused)
+static void test_visitor_out_null(TestOutputVisitorData *data,
+                                  const void *unused)
 {
     QObject *arg;
+    QDict *qdict;
+    QObject *nil;
 
+    visit_start_struct(data->ov, NULL, NULL, 0, &error_abort);
+    visit_type_null(data->ov, "a", &error_abort);
+    visit_check_struct(data->ov, &error_abort);
+    visit_end_struct(data->ov);
     arg = qmp_output_get_qobject(data->qov);
-    g_assert(qobject_type(arg) == QTYPE_QNULL);
-    /* Check that qnull reference counting is sane */
-    g_assert(arg->refcnt == 2);
+    g_assert(qobject_type(arg) == QTYPE_QDICT);
+    qdict = qobject_to_qdict(arg);
+    g_assert_cmpint(qdict_size(qdict), ==, 1);
+    nil = qdict_get(qdict, "a");
+    g_assert(nil);
+    g_assert(qobject_type(nil) == QTYPE_QNULL);
     qobject_decref(arg);
 }
 
@@ -839,8 +860,8 @@ int main(int argc, char **argv)
                             &out_visitor_data, test_visitor_out_union_flat);
     output_visitor_test_add("/visitor/output/alternate",
                             &out_visitor_data, test_visitor_out_alternate);
-    output_visitor_test_add("/visitor/output/empty",
-                            &out_visitor_data, test_visitor_out_empty);
+    output_visitor_test_add("/visitor/output/null",
+                            &out_visitor_data, test_visitor_out_null);
     output_visitor_test_add("/visitor/output/native_list/int",
                             &out_visitor_data,
                             test_visitor_out_native_list_int);

tests/test-string-input-visitor.c

@@ -63,6 +63,13 @@ static void test_visitor_in_int(TestInputVisitorData *data,
     visit_type_int(v, NULL, &res, &err);
     g_assert(!err);
     g_assert_cmpint(res, ==, value);
+
+    visitor_input_teardown(data, unused);
+
+    v = visitor_input_test_init(data, "not an int");
+
+    visit_type_int(v, NULL, &res, &err);
+    error_free_or_abort(&err);
 }
 
 static void test_visitor_in_intList(TestInputVisitorData *data,
@@ -70,6 +77,7 @@ static void test_visitor_in_intList(TestInputVisitorData *data,
 {
     int64_t value[] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 20};
     int16List *res = NULL, *tmp;
+    Error *err = NULL;
     Visitor *v;
     int i = 0;
 
@@ -84,12 +92,15 @@ static void test_visitor_in_intList(TestInputVisitorData *data,
     }
     g_assert(!tmp);
 
-    tmp = res;
-    while (tmp) {
-        res = res->next;
-        g_free(tmp);
-        tmp = res;
-    }
+    qapi_free_int16List(res);
+
+    visitor_input_teardown(data, unused);
+
+    v = visitor_input_test_init(data, "not an int list");
+
+    visit_type_int16List(v, NULL, &res, &err);
+    error_free_or_abort(&err);
+    g_assert(!res);
 }
 
 static void test_visitor_in_bool(TestInputVisitorData *data,

tests/test-visitor-serialization.c

@@ -1038,7 +1038,7 @@ static void qmp_deserialize(void **native_out, void *datap,
     obj = qobject_from_json(qstring_get_str(output_json));
 
     QDECREF(output_json);
-    d->qiv = qmp_input_visitor_new(obj);
+    d->qiv = qmp_input_visitor_new(obj, true);
     qobject_decref(obj_orig);
     qobject_decref(obj);
     visit(qmp_input_get_visitor(d->qiv), native_out, errp);

util/qemu-sockets.c

@@ -1145,7 +1145,7 @@ void qapi_copy_SocketAddress(SocketAddress **p_dest,
         return;
     }
 
-    qiv = qmp_input_visitor_new(obj);
+    qiv = qmp_input_visitor_new(obj, true);
     iv = qmp_input_get_visitor(qiv);
     visit_type_SocketAddress(iv, NULL, p_dest, &error_abort);
     qmp_input_visitor_cleanup(qiv);