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: Peter Maydell <peter.maydell@linaro.org>

8 files changed, 560 insertions, 49 deletions

diff --git a/include/qapi/dealloc-visitor.h b/include/qapi/dealloc-visitor.h
index cf4c36d2d3..45b06b248c 100644
--- a/include/qapi/dealloc-visitor.h
+++ b/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);
 
diff --git a/include/qapi/opts-visitor.h b/include/qapi/opts-visitor.h
index fd48c14ec8..ae1bf7cf51 100644
--- a/include/qapi/opts-visitor.h
+++ b/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);
diff --git a/include/qapi/qmp-input-visitor.h b/include/qapi/qmp-input-visitor.h
index 3ed499cc42..b0624d8683 100644
--- a/include/qapi/qmp-input-visitor.h
+++ b/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);
 
diff --git a/include/qapi/qmp/dispatch.h b/include/qapi/qmp/dispatch.h
index 495520994c..5609946a16 100644
--- a/include/qapi/qmp/dispatch.h
+++ b/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;
diff --git a/include/qapi/string-input-visitor.h b/include/qapi/string-input-visitor.h
index 089243c09e..7b76c2b9e3 100644
--- a/include/qapi/string-input-visitor.h
+++ b/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);
 
diff --git a/include/qapi/string-output-visitor.h b/include/qapi/string-output-visitor.h
index d99717f650..e10522a35b 100644
--- a/include/qapi/string-output-visitor.h
+++ b/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);
 
diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
index 2bd8f292b2..145afd03e7 100644
--- a/include/qapi/visitor-impl.h
+++ b/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
diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index 9a8d0105fb..4d12167bdc 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -16,8 +16,199 @@
 
 #include "qapi/qmp/qobject.h"
 
+/*
+ * The QAPI schema defines both a set of C data types, and a QMP wire
+ * format.  QAPI objects can contain references to other QAPI objects,
+ * resulting in a directed acyclic graph.  QAPI also generates visitor
+ * functions to walk these graphs.  This file represents the interface
+ * for doing work at each node 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 (QMP,
+ * string, and QemuOpts) parse an external representation and build
+ * the corresponding QAPI graph, output visitors (QMP and string) take
+ * a completed QAPI graph and generate an external representation, and
+ * the dealloc visitor can take a QAPI graph (possibly partially
+ * constructed) 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, T obj, Error **errp);
+ *
+ * where T is FOO for scalar types, and FOO * otherwise.  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 ignored; 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.
+ *
+ * FIXME: Clients must pass NULL for @name when visiting a member of a
+ * list, but this leads to poor error messages; it might be nicer to
+ * require a non-NULL name such as "key.0" for '{ "key": [ "value" ]
+ * }' if an error is encountered on "value" (or to have the visitor
+ * core auto-generate the nicer name).
+ *
+ * The visit_type_FOO() functions expect a non-null @obj argument;
+ * they allocate *@obj during input visits, leave it unchanged on
+ * output visits, and recursively free any resources during a dealloc
+ * visit.  Each function also takes the customary @errp argument (see
+ * qapi/error.h for details), for reporting any errors (such as if a
+ * member @name is not present, or is present but not the specified
+ * type).
+ *
+ * If an error is detected during visit_type_FOO() with an input
+ * visitor, then *@obj will be NULL for pointer types, and left
+ * unchanged for scalar types.  Using an output visitor with an
+ * incomplete object has undefined behavior (other than a special case
+ * for visit_type_str() treating NULL like ""), while the dealloc
+ * visitor safely handles incomplete objects.  Since input visitors
+ * never produce an incomplete object, such an object is possible only
+ * by manual construction.
+ *
+ * For the QAPI object types (structs, unions, and alternates), there
+ * is an additional 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() in that @obj may 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, typical input usage involves:
+ *
+ * <example>
+ *  Foo *f;
+ *  Error *err = NULL;
+ *  Visitor *v;
+ *
+ *  v = ...obtain input visitor...
+ *  visit_type_Foo(v, NULL, &f, &err);
+ *  if (err) {
+ *      ...handle error...
+ *  } else {
+ *      ...use f...
+ *  }
+ *  ...clean up v...
+ *  qapi_free_Foo(f);
+ * </example>
+ *
+ * For a list, it is:
+ * <example>
+ *  FooList *l;
+ *  Error *err = NULL;
+ *  Visitor *v;
+ *
+ *  v = ...obtain input visitor...
+ *  visit_type_FooList(v, NULL, &l, &err);
+ *  if (err) {
+ *      ...handle error...
+ *  } else {
+ *      for ( ; l; l = l->next) {
+ *          ...use l->value...
+ *      }
+ *  }
+ *  ...clean up v...
+ *  qapi_free_FooList(l);
+ * </example>
+ *
+ * Similarly, typical output usage is:
+ *
+ * <example>
+ *  Foo *f = ...obtain populated object...
+ *  Error *err = NULL;
+ *  Visitor *v;
+ *
+ *  v = ...obtain output visitor...
+ *  visit_type_Foo(v, NULL, &f, &err);
+ *  if (err) {
+ *      ...handle error...
+ *  }
+ *  ...clean up v...
+ * </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.  Similarly, the
+ * visit_is_input() helper makes it possible to write code that is
+ * visitor-agnostic everywhere except for cleanup.  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 out;
+ *  }
+ *  visit_start_list(v, "list", NULL, 0, &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);
+ *  if (!err) {
+ *      visit_check_struct(v, &err);
+ *  }
+ * outobj:
+ *  visit_end_struct(v);
+ * out:
+ *  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,35 +216,139 @@ 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 @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
+ * *@obj.  @obj may also be NULL for a virtual walk, in which case
+ * @size is ignored.
+ *
+ * @errp obeys typical error usage, and reports failures such as a
+ * member @name is not present, or present but not an object.  On
+ * error, input visitors set *@obj 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);
-void visit_end_struct(Visitor *v, Error **errp);
 
-void visit_start_list(Visitor *v, const char *name, Error **errp);
-GenericList *visit_next_list(Visitor *v, GenericList **list, size_t size);
+/*
+ * Prepare for completing an object visit.
+ *
+ * @errp obeys typical error usage, and reports failures such as
+ * unparsed keys remaining in the input stream.
+ *
+ * Should be called prior to visit_end_struct() if all other
+ * intermediate visit steps were successful, to allow the visitor one
+ * last chance to report errors.  May be skipped on a cleanup path,
+ * where there is no need to check for further errors.
+ */
+void visit_check_struct(Visitor *v, Error **errp);
+
+/*
+ * Complete an object visit started earlier.
+ *
+ * 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 early
+ * behaves as if this was implicitly called.
+ */
+void visit_end_struct(Visitor *v);
+
+
+/*** Visiting lists ***/
+
+/*
+ * Start visiting a list.
+ *
+ * @name expresses the relationship of this list to its parent
+ * container; see the general description of @name above.
+ *
+ * @list must be non-NULL for a real walk, in which case @size
+ * determines how much memory an input visitor will allocate into
+ * *@list (at least sizeof(GenericList)).  Some visitors also allow
+ * @list to be NULL for a virtual walk, in which case @size is
+ * ignored.
+ *
+ * @errp obeys typical error usage, and reports failures such as a
+ * member @name is not present, or present but not a list.  On error,
+ * input visitors set *@list to NULL.
+ *
+ * After visit_start_list() succeeds, the caller may visit its members
+ * one after the other.  A real visit (where @obj is non-NULL) uses
+ * visit_next_list() for traversing the linked list, while a virtual
+ * visit (where @obj is NULL) 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, GenericList **list,
+                      size_t size, Error **errp);
+
+/*
+ * Iterate over a GenericList during a non-virtual list visit.
+ *
+ * @size represents the size of a linked list node (at least
+ * sizeof(GenericList)).
+ *
+ * @tail must not be NULL; on the first call, @tail is the value of
+ * *list after visit_start_list(), and on subsequent calls @tail must
+ * be the previously returned value.  Should be called in a loop until
+ * a NULL return or error occurs; for each non-NULL return, the caller
+ * then calls 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 @tail->value.
+ */
+GenericList *visit_next_list(Visitor *v, GenericList *tail, 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 early
+ * behaves 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.
  *
- * @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
- * (*@obj)->type.  Other visitors will leave @obj unchanged.
+ * @obj must not be NULL. Input visitors use @size to determine how
+ * much memory to allocate into *@obj, then determine the qtype of the
+ * next thing to be visited, stored in (*@obj)->type.  Other visitors
+ * will leave @obj unchanged.
  *
  * 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 +357,202 @@ 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 early
+ * behaves 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 *@present according to whether the
- * corresponding visit_type_*() needs calling; for other visitors,
- * leave *@present unchanged.  Return *@present 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 *@present according to input; other visitors
+ * leave it unchanged.  In either case, return *@present 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.
+ *
+ * @obj must be non-NULL.  Input visitors parse input and set *@obj to
+ * the enumeration value, leaving @obj unchanged on error; other
+ * visitors use *@obj but leave it unchanged.
+ *
+ * Currently, all input visitors parse text input, and all output
+ * visitors produce text output.  The mapping between enumeration
+ * values and strings is done by the visitor core, using @strings; it
+ * should be the ENUM_lookup array from visit-types.h.
+ *
+ * May call visit_type_str() under the hood, and the enum visit may
+ * fail even if the corresponding string visit succeeded; this implies
+ * that visit_type_str() must have no unwelcome side effects.
+ */
 void visit_type_enum(Visitor *v, const char *name, int *obj,
                      const char *const strings[], Error **errp);
+
+/*
+ * Check if visitor is an input visitor.
+ */
+bool visit_is_input(Visitor *v);
+
+/*** 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 *@obj to the value;
+ * other visitors will leave *@obj 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 *@obj to the value;
+ * other visitors will leave *@obj 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 *@obj to the value
+ * (never NULL).  Other visitors leave *@obj unchanged, and commonly
+ * treat NULL like "".
+ *
+ * It is safe to cast away const when preparing a (const char *) value
+ * into @obj for use by an output visitor.
+ *
+ * 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 (i.e. double) 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 *@obj to the value;
+ * other visitors will leave *@obj unchanged.  Visitors should
+ * document if infinity or NaN are not permitted.
+ */
 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 *@obj to the value;
+ * other visitors will leave *@obj unchanged.  *@obj must be non-NULL
+ * for output visitors.
+ */
 void visit_type_any(Visitor *v, const char *name, QObject **obj, Error **errp);
 
+/*
+ * Visit a JSON null value.
+ *
+ * @name expresses the relationship of the null value to its parent
+ * container; see the general description of @name above.
+ *
+ * Unlike all other visit_type_* functions, no obj parameter is
+ * needed; rather, this is a witness that an explicit null value is
+ * expected rather than any other type.
+ */
+void visit_type_null(Visitor *v, const char *name, Error **errp);
+
 #endif