[Qemu-devel] [PATCH v14 08/19] qapi: Document visitor interfaces, add assertions

[Eric Blake]

The visitor interface for mapping between QObject/QemuOpts/string
and QAPI is scandalously under-documented, making changes to visitor
core, individual visitors, and users of visitors difficult to
coordinate.  Among other questions: when is it safe to pass NULL,
vs. when a string must be provided; which visitors implement which
callbacks; the difference between concrete and virtual visits.

Correct this by retrofitting proper contracts, and document where some
of the interface warts remain (for example, we may want to modify
visit_end_* to require the same 'obj' as the visit_start counterpart,
so the dealloc visitor can be simplified).  Later patches in this
series will tackle some, but not all, of these warts.

Add assertions to (partially) enforce the contract.  Some of these
were only made possible by recent cleanup commits.

Signed-off-by: Eric Blake <address@hidden>

---
v14: rebase to master context
v13: minor wording tweaks for consistency
v12: major rework based on Markus' comments, drop R-b
[no v10, v11]
v9: no change
v8: rebase to 'name' motion
v7: retitle; more wording changes, add asserts to enforce the
wording, place later in series to rebase on fixes that would
otherwise trip the new assertions
v6: mention that input visitors blindly assign over *obj; wording
improvements
---
 include/qapi/visitor.h               | 433 +++++++++++++++++++++++++++++++++--
 include/qapi/visitor-impl.h          |  42 +++-
 include/qapi/dealloc-visitor.h       |   4 +
 include/qapi/opts-visitor.h          |   4 +
 include/qapi/qmp-input-visitor.h     |   8 +
 include/qapi/string-input-visitor.h  |   4 +
 include/qapi/string-output-visitor.h |   4 +
 qapi/qapi-visit-core.c               |  19 +-
 8 files changed, 494 insertions(+), 24 deletions(-)

diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index 9a8d010..faf67d2 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -16,8 +16,185 @@

 #include "qapi/qmp/qobject.h"

+/*
+ * The QAPI schema defines both a set of C data types, and a QMP wire
+ * format.  A QAPI object is formed as a directed acyclic graph of
+ * QAPI values.  QAPI also generates visitor functions to walk these
+ * graphs.  This file represents the interface for doing work at each
+ * point of a QAPI graph; it can also be used for a virtual walk,
+ * where there is no actual QAPI C struct.
+ *
+ * There are three kinds of visitor classes: input visitors parse an
+ * external representation and allocate the corresponding QAPI graph
+ * (QMP, string, and QemuOpts), output visitors take a completed QAPI
+ * graph and generate an external representation (QMP and string), and
+ * the dealloc visitor can take a (partial) QAPI graph and recursively
+ * free its resources.  While the dealloc and QMP input/output
+ * visitors are general, the string and QemuOpts visitors have some
+ * implementation limitations; see the documentation for each visitor
+ * for more details on what it supports.  Also, see visitor-impl.h for
+ * the callback contracts implemented by each visitor, and
+ * docs/qapi-code-gen.txt for more about the QAPI code generator.
+ *
+ * All QAPI types have a corresponding function with a signature
+ * roughly compatible with this:
+ *
+ * void visit_type_FOO(Visitor *v, const char *name, void *obj, Error **errp);
+ *
+ * except that address@hidden is typed correctly as a pointer or scalar,
+ * depending on the type of FOO.  The scalar visitors are declared
+ * here; the remaining visitors are generated in qapi-visit.h.
+ *
+ * The @name parameter of visit_type_FOO() describes the relation
+ * between this QAPI value and its parent container.  When visiting
+ * the root of a tree, @name is usually ignored (although some
+ * visitors require it to be NULL); when visiting a member of an
+ * object, @name is the key associated with the value; and when
+ * visiting a member of a list, @name is NULL.
+ *
+ * The visit_type_FOO() functions expect a non-NULL @obj argument;
+ * they allocate address@hidden during input visits, leave it unchanged on
+ * output visits, and recursively free any resources during a dealloc
+ * visit.  Each function also has an @errp argument which may be NULL
+ * to ignore errors, or point to a NULL Error object on entry for
+ * reporting any errors (such as if a member @name is not present, or
+ * is present but not the specified type).
+ *
+ * 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).
+ *
+ * FIXME: At present, input visitors may allocate an incomplete address@hidden
+ * even when visit_type_FOO() reports an error.  Using an output
+ * visitor with an incomplete object has undefined behavior; callers
+ * must call qapi_free_FOO() (which uses the dealloc visitor, and
+ * safely handles an incomplete object) to avoid a memory leak.
+ *
+ * Likewise, the QAPI object types (structs, unions, and alternates)
+ * have a generated function in qapi-visit.h compatible with:
+ *
+ * void visit_type_FOO_members(Visitor *v, FOO *obj, Error **errp);
+ *
+ * for visiting the members of a type without also allocating the QAPI
+ * struct.
+ *
+ * Additionally, in qapi-types.h, all QAPI pointer types (structs,
+ * unions, alternates, and lists) have a generated function compatible
+ * with:
+ *
+ * void qapi_free_FOO(FOO *obj);
+ *
+ * which behaves like free() (@obj can be NULL); because of these
+ * functions, the dealloc visitor is seldom used directly outside of
+ * generated code.  QAPI types can also inherit from a base class;
+ * when this happens, a function is generated for easily going from
+ * the derived type to the base type:
+ *
+ * BASE *qapi_CHILD_base(CHILD *obj);
+ *
+ * For a real QAPI struct, a typical input life cycle is thus:
+ *
+ * <example>
+ *  Foo *f;
+ *  FooList *l;
+ *  Error *err = NULL;
+ *  Visitor *v;
+ *
+ *  v = ...obtain input visitor...
+ *  visit_type_Foo(v, NULL, &f, &err);
+ *  if (err) {
+ *      qapi_free_Foo(f);
+ *      ...handle error...
+ *  } else {
+ *      ...use f...
+ *  }
+ *  v = ...obtain input visitor...
+ *  visit_type_FooList(v, NULL, &l, &err);
+ *  if (err) {
+ *      qapi_free_FooList(l);
+ *      ...handle error...
+ *  } else {
+ *      while (l) {
+ *          ...use l->value...
+ *          l = l->next;
+ *      }
+ *  }
+ *  ...clean up v...
+ * </example>
+ *
+ * Similarly, a typical output life cycle is:
+ *
+ * <example>
+ *  Foo *f = g_new0(Foo, 1);
+ *  Error *err = NULL;
+ *  Visitor *v;
+ *
+ *  ...populate f...
+ *  v = ...obtain output visitor...
+ *  visit_type_Foo(v, NULL, &f, &err);
+ *  if (err) {
+ *      ...handle error...
+ *  }
+ *  qapi_free_Foo(f);
+ * </example>
+ *
+ * When visiting a real QAPI struct, this file provides several
+ * helpers that rely on in-tree information to control the walk:
+ * visit_optional() for the 'has_member' field associated with
+ * optional 'member' in the C struct; and visit_next_list() for
+ * advancing through a FooList linked list.  Only the generated
+ * visit_type functions need to use these helpers.
+ *
+ * It is also possible to use the visitors to do a virtual walk, where
+ * no actual QAPI struct is present.  In this situation, decisions
+ * about what needs to be walked are made by the calling code, and
+ * structured visits are split between pairs of start and end methods
+ * (where the end method must be called if the start function
+ * succeeded, even if an intermediate visit encounters an error).
+ * Thus, a virtual walk corresponding to '{ "list": [1, 2] }' looks
+ * like:
+ *
+ * <example>
+ *  Visitor *v;
+ *  Error *err = NULL;
+ *  int value;
+ *
+ *  v = ...obtain visitor...
+ *  visit_start_struct(v, NULL, NULL, 0, &err);
+ *  if (err) {
+ *      goto outobj;
+ *  }
+ *  visit_start_list(v, "list", &err);
+ *  if (err) {
+ *      goto outobj;
+ *  }
+ *  value = 1;
+ *  visit_type_int(v, NULL, &value, &err);
+ *  if (err) {
+ *      goto outlist;
+ *  }
+ *  value = 2;
+ *  visit_type_int(v, NULL, &value, &err);
+ *  if (err) {
+ *      goto outlist;
+ *  }
+ * outlist:
+ *  visit_end_list(v);
+ * outobj:
+ *  error_propagate(errp, err);
+ *  err = NULL;
+ *  visit_end_struct(v, &err);
+ *  error_propagate(errp, err);
+ *  ...clean up v...
+ * </example>
+ */
+
+/* === Useful types */
+
 /* This struct is layout-compatible with all other *List structs
- * created by the qapi generator.  It is used as a typical
+ * created by the QAPI generator.  It is used as a typical
  * singly-linked list. */
 typedef struct GenericList {
     struct GenericList *next;
@@ -25,26 +202,117 @@ typedef struct GenericList {
 } GenericList;

 /* This struct is layout-compatible with all Alternate types
- * created by the qapi generator. */
+ * created by the QAPI generator. */
 typedef struct GenericAlternate {
     QType type;
     char padding[];
 } GenericAlternate;

+/* === Visiting structures */
+
+/*
+ * Start visiting an object value @obj (struct or union).
+ *
+ * @name expresses the relationship of this object to its parent
+ * container; see the general description of @name above.
+ *
+ * @obj must be non-NULL for a real walk, in which case @size
+ * determines how much memory an input visitor will allocate into
+ * address@hidden  @obj may also be NULL for a virtual walk, in which case
+ * @size is ignored.
+ *
+ * @errp must be NULL-initialized, and is set if an error is detected
+ * (such as if a member @name is not present, or is present but not an
+ * object).  On error, input visitors set address@hidden to NULL.
+ *
+ * After visit_start_struct() succeeds, the caller may visit its
+ * members one after the other, passing the member's name and address
+ * within the struct.  Finally, visit_end_struct() needs to be called
+ * to clean up, even if intermediate visits fail.  See the examples
+ * above.
+ *
+ * FIXME Should this be named visit_start_object, since it is also
+ * used for QAPI unions, and maps to JSON objects?
+ */
 void visit_start_struct(Visitor *v, const char *name, void **obj,
                         size_t size, Error **errp);
+
+/*
+ * Complete an object visit started earlier.
+ *
+ * @errp must be NULL-initialized, and is set if an error is detected
+ * (such as unparsed keys remaining in the input stream).
+ *
+ * Must be called after any successful use of visit_start_struct(),
+ * even if intermediate processing was skipped due to errors, to allow
+ * the backend to release any resources.  Destroying the visitor may
+ * behave as if this was implicitly called.
+ */
 void visit_end_struct(Visitor *v, Error **errp);

+
+/* === Visiting lists */
+
+/*
+ * Start visiting a list.
+ *
+ * @name expresses the relationship of this list to its parent
+ * container; see the general description of @name above.
+ *
+ * @errp must be NULL-initialized, and is set if an error is detected
+ * (such as if a member @name is not present, or is present but not a
+ * list).
+ *
+ * After visit_start_list() succeeds, the caller may visit its members
+ * one after the other.  A real visit uses visit_next_list() for
+ * traversing the linked list, while a virtual visit uses other means.
+ * For each list element, call the appropriate visit_type_FOO() with
+ * name set to NULL and obj set to the address of the value member of
+ * the list element.  Finally, visit_end_list() needs to be called to
+ * clean up, even if intermediate visits fail.  See the examples
+ * above.
+ */
 void visit_start_list(Visitor *v, const char *name, Error **errp);
+
+/*
+ * Iterate over a GenericList during a list visit.
+ *
+ * @size represents the size of a linked list node.
+ *
+ * @list must not be NULL; on the first call, @list contains the
+ * address of the list head, and on subsequent calls address@hidden must be
+ * the previously returned value.  Must be called in a loop until a
+ * NULL return or error occurs; for each non-NULL return, the caller
+ * must then call the appropriate visit_type_*() for the element type
+ * of the list, with that function's name parameter set to NULL and
+ * obj set to the address of (address@hidden)->value.
+ *
+ * FIXME: This interface is awkward; it requires all callbacks to
+ * track whether it is the first or a subsequent call.  A better
+ * interface would pass the head of the list through
+ * visit_start_list().
+ */
 GenericList *visit_next_list(Visitor *v, GenericList **list, size_t size);
+
+/*
+ * Complete a list visit started earlier.
+ *
+ * Must be called after any successful use of visit_start_list(),
+ * even if intermediate processing was skipped due to errors, to allow
+ * the backend to release any resources.  Destroying the visitor may
+ * behave as if this was implicitly called.
+ */
 void visit_end_list(Visitor *v);

+
+/* === Visiting alternates */
+
 /*
- * Start the visit of an alternate @obj with the given @size.
+ * Start the visit of an alternate @obj with the given @size (at least
+ * sizeof(GenericAlternate)).
  *
- * @name specifies the relationship to the containing struct (ignored
- * for a top level visit, the name of the key if this alternate is
- * part of an object, or NULL if this alternate is part of a list).
+ * @name expresses the relationship of this alternate to its parent
+ * container; see the general description of @name above.
  *
  * @obj must not be NULL. Input visitors will allocate @obj and
  * determine the qtype of the next thing to be visited, stored in
@@ -52,8 +320,8 @@ void visit_end_list(Visitor *v);
  *
  * If @promote_int, treat integers as QTYPE_FLOAT.
  *
- * If successful, this must be paired with visit_end_alternate(), even
- * if visiting the contents of the alternate fails.
+ * If successful, this must be paired with visit_end_alternate() to
+ * clean up, even if visiting the contents of the alternate fails.
  */
 void visit_start_alternate(Visitor *v, const char *name,
                            GenericAlternate **obj, size_t size,
@@ -62,46 +330,181 @@ void visit_start_alternate(Visitor *v, const char *name,
 /*
  * Finish visiting an alternate type.
  *
- * Must be called after a successful visit_start_alternate(), even if
- * an error occurred in the meantime.
+ * Must be called after any successful use of visit_start_alternate(),
+ * even if intermediate processing was skipped due to errors, to allow
+ * the backend to release any resources.  Destroying the visitor may
+ * behave as if this was implicitly called.
  *
  * TODO: Should all the visit_end_* interfaces take obj parameter, so
  * that dealloc visitor need not track what was passed in visit_start?
  */
 void visit_end_alternate(Visitor *v);

-/**
- * Check if an optional member @name of an object needs visiting.
- * For input visitors, set address@hidden according to whether the
- * corresponding visit_type_*() needs calling; for other visitors,
- * leave address@hidden unchanged.  Return address@hidden for convenience.
+
+/* === Other helpers */
+
+/*
+ * Does optional struct member @name need visiting?
+ *
+ * @name must not be NULL.  This function is only useful between
+ * visit_start_struct() and visit_end_struct(), since only objects
+ * have optional keys.
+ *
+ * @present points to the address of the optional member's has_ flag.
+ *
+ * Input visitors set address@hidden according to input; other visitors
+ * leave it unchanged.  In either case, return address@hidden for
+ * convenience.
  */
 bool visit_optional(Visitor *v, const char *name, bool *present);

+/*
+ * Visit an enum value.
+ *
+ * @name expresses the relationship of this enum to its parent
+ * container; see the general description of @name above.
+ *
+ * @strings expresses the mapping between C enum values and QAPI enum
+ * names; it should be the ENUM_lookup array from visit-types.h.
+ *
+ * @obj must be non-NULL.  For input visitors, parse a string and set
+ * address@hidden to the numeric value of the enum type using @strings as the
+ * mapping, leaving @obj unchanged on error; other visitors leave
+ * address@hidden unchanged.  Output visitors use address@hidden to reverse 
the mapping
+ * and visit the appropriate output string.
+ */
 void visit_type_enum(Visitor *v, const char *name, int *obj,
                      const char *const strings[], Error **errp);
+
+
+/* === Visiting built-in types */
+
+/*
+ * Visit an integer value.
+ *
+ * @name expresses the relationship of this integer to its parent
+ * container; see the general description of @name above.
+ *
+ * @obj must be non-NULL.  Input visitors set address@hidden to the value;
+ * other visitors will leave address@hidden unchanged.
+ */
 void visit_type_int(Visitor *v, const char *name, int64_t *obj, Error **errp);
+
+/*
+ * Visit a uint8_t value.
+ * Like visit_type_int(), except clamps the value to uint8_t range.
+ */
 void visit_type_uint8(Visitor *v, const char *name, uint8_t *obj,
                       Error **errp);
+
+/*
+ * Visit a uint16_t value.
+ * Like visit_type_int(), except clamps the value to uint16_t range.
+ */
 void visit_type_uint16(Visitor *v, const char *name, uint16_t *obj,
                        Error **errp);
+
+/*
+ * Visit a uint32_t value.
+ * Like visit_type_int(), except clamps the value to uint32_t range.
+ */
 void visit_type_uint32(Visitor *v, const char *name, uint32_t *obj,
                        Error **errp);
+
+/*
+ * Visit a uint64_t value.
+ * Like visit_type_int(), except clamps the value to uint64_t range,
+ * that is, ensures it is unsigned.
+ */
 void visit_type_uint64(Visitor *v, const char *name, uint64_t *obj,
                        Error **errp);
+
+/*
+ * Visit an int8_t value.
+ * Like visit_type_int(), except clamps the value to int8_t range.
+ */
 void visit_type_int8(Visitor *v, const char *name, int8_t *obj, Error **errp);
+
+/*
+ * Visit an int16_t value.
+ * Like visit_type_int(), except clamps the value to int16_t range.
+ */
 void visit_type_int16(Visitor *v, const char *name, int16_t *obj,
                       Error **errp);
+
+/*
+ * Visit an int32_t value.
+ * Like visit_type_int(), except clamps the value to int32_t range.
+ */
 void visit_type_int32(Visitor *v, const char *name, int32_t *obj,
                       Error **errp);
+
+/*
+ * Visit an int64_t value.
+ * Identical to visit_type_int().
+ */
 void visit_type_int64(Visitor *v, const char *name, int64_t *obj,
                       Error **errp);
+
+/*
+ * Visit a uint64_t value.
+ * Like visit_type_uint64(), except that some visitors may choose to
+ * recognize additional syntax, such as suffixes for easily scaling
+ * values.
+ */
 void visit_type_size(Visitor *v, const char *name, uint64_t *obj,
                      Error **errp);
+
+/*
+ * Visit a boolean value.
+ *
+ * @name expresses the relationship of this boolean to its parent
+ * container; see the general description of @name above.
+ *
+ * @obj must be non-NULL.  Input visitors set address@hidden to the value;
+ * other visitors will leave address@hidden unchanged.
+ */
 void visit_type_bool(Visitor *v, const char *name, bool *obj, Error **errp);
+
+/*
+ * Visit a string value.
+ *
+ * @name expresses the relationship of this string to its parent
+ * container; see the general description of @name above.
+ *
+ * @obj must be non-NULL.  Input visitors set address@hidden to the value
+ * (never NULL).  Other visitors leave address@hidden unchanged, and commonly
+ * treat NULL like "".
+ *
+ * Note that using an output visitor along with a (const char *) value
+ * requires casting away const when computing @obj.
+ *
+ * FIXME: Callers that try to output NULL *obj should not be allowed.
+ */
 void visit_type_str(Visitor *v, const char *name, char **obj, Error **errp);
+
+/*
+ * Visit a number value.
+ *
+ * @name expresses the relationship of this number to its parent
+ * container; see the general description of @name above.
+ *
+ * @obj must be non-NULL.  Input visitors set address@hidden to the value;
+ * other visitors will leave address@hidden unchanged.
+ */
 void visit_type_number(Visitor *v, const char *name, double *obj,
                        Error **errp);
+
+/*
+ * Visit an arbitrary value.
+ *
+ * @name expresses the relationship of this value to its parent
+ * container; see the general description of @name above.
+ *
+ * @obj must be non-NULL.  Input visitors set address@hidden to the value;
+ * other visitors will leave address@hidden unchanged.  address@hidden must be 
non-NULL
+ * for output visitors.
+ */
 void visit_type_any(Visitor *v, const char *name, QObject **obj, Error **errp);

 #endif
diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
index 228a2a6..d967b18 100644
--- a/include/qapi/visitor-impl.h
+++ b/include/qapi/visitor-impl.h
@@ -14,6 +14,16 @@

 #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.  */
@@ -25,18 +35,24 @@ typedef enum 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);
+
+    /* Must be set to visit structs.  */
     void (*end_struct)(Visitor *v, Error **errp);

+    /* Must be set.  */
     void (*start_list)(Visitor *v, const char *name, Error **errp);
-    /* Must be set */
+
+    /* Must be set.  */
     GenericList *(*next_list)(Visitor *v, GenericList **list, size_t size);
-    /* Must be set */
+
+    /* 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);
@@ -44,24 +60,34 @@ struct Visitor
     /* Optional, needed for dealloc visitor.  */
     void (*end_alternate)(Visitor *v);

-    /* 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().  */
     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 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);

     /* Must be set.  */
diff --git a/include/qapi/dealloc-visitor.h b/include/qapi/dealloc-visitor.h
index cf4c36d..7aa8293 100644
--- a/include/qapi/dealloc-visitor.h
+++ b/include/qapi/dealloc-visitor.h
@@ -18,6 +18,10 @@

 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);

diff --git a/include/qapi/opts-visitor.h b/include/qapi/opts-visitor.h
index fd48c14..2002e37 100644
--- a/include/qapi/opts-visitor.h
+++ b/include/qapi/opts-visitor.h
@@ -29,6 +29,10 @@ 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 yet implement support for visiting
+ * QAPI alternates, numbers (other than integers), or arbitrary
+ * QTypes.
  */
 OptsVisitor *opts_visitor_new(const QemuOpts *opts);
 void opts_visitor_cleanup(OptsVisitor *nv);
diff --git a/include/qapi/qmp-input-visitor.h b/include/qapi/qmp-input-visitor.h
index 3ed499c..d75ff98 100644
--- a/include/qapi/qmp-input-visitor.h
+++ b/include/qapi/qmp-input-visitor.h
@@ -19,6 +19,14 @@

 typedef struct QmpInputVisitor QmpInputVisitor;

+/*
+ * FIXME: When visiting a QDict, passing a non-NULL @name for the
+ * first visit_type_FOO() when the root is a QDict will find that
+ * particular key within the QDict.  In the future, the contract may
+ * be tightened to require visit_start_struct() with ignored @name as
+ * the first visit; in the meantime, the first visit is safest when
+ * using NULL for @name.
+ */
 QmpInputVisitor *qmp_input_visitor_new(QObject *obj);
 QmpInputVisitor *qmp_input_visitor_new_strict(QObject *obj);

diff --git a/include/qapi/string-input-visitor.h 
b/include/qapi/string-input-visitor.h
index 089243c..4c8d1ea 100644
--- a/include/qapi/string-input-visitor.h
+++ b/include/qapi/string-input-visitor.h
@@ -17,6 +17,10 @@

 typedef struct StringInputVisitor StringInputVisitor;

+/*
+ * The string input visitor does not yet implement support for
+ * visiting QAPI structs, alternates, or arbitrary QTypes.
+ */
 StringInputVisitor *string_input_visitor_new(const char *str);
 void string_input_visitor_cleanup(StringInputVisitor *v);

diff --git a/include/qapi/string-output-visitor.h 
b/include/qapi/string-output-visitor.h
index d99717f..094a11e 100644
--- a/include/qapi/string-output-visitor.h
+++ b/include/qapi/string-output-visitor.h
@@ -17,6 +17,10 @@

 typedef struct StringOutputVisitor StringOutputVisitor;

+/*
+ * The string output visitor does not yet implement support for
+ * visiting QAPI structs, alternates, or arbitrary QTypes.
+ */
 StringOutputVisitor *string_output_visitor_new(bool human);
 void string_output_visitor_cleanup(StringOutputVisitor *v);

diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index 3cd7edc..71f3b5d 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -23,6 +23,10 @@
 void visit_start_struct(Visitor *v, const char *name, void **obj,
                         size_t size, Error **errp)
 {
+    if (obj) {
+        assert(size);
+        assert(v->type != VISITOR_OUTPUT || *obj);
+    }
     v->start_struct(v, name, obj, size, errp);
 }

@@ -74,6 +78,7 @@ bool visit_optional(Visitor *v, const char *name, bool 
*present)

 void visit_type_int(Visitor *v, const char *name, int64_t *obj, Error **errp)
 {
+    assert(obj);
     v->type_int64(v, name, obj, errp);
 }

@@ -121,6 +126,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);
 }

@@ -168,12 +174,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 {
@@ -183,22 +191,31 @@ 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)
 {
+    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, errp);
 }

 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)
 {
+    assert(obj);
+    assert(v->type != VISITOR_OUTPUT || *obj);
     v->type_any(v, name, obj, errp);
 }

@@ -252,7 +269,7 @@ static void input_type_enum(Visitor *v, const char *name, 
int *obj,
 void visit_type_enum(Visitor *v, const char *name, int *obj,
                      const char *const strings[], Error **errp)
 {
-    assert(strings);
+    assert(obj && strings);
     if (v->type == VISITOR_INPUT) {
         input_type_enum(v, name, obj, strings, errp);
     } else if (v->type == VISITOR_OUTPUT) {
-- 
2.5.5