[Qemu-devel] [PATCH RFC v4 19/29] qapi: Document visitor interfaces

[Eric Blake]

The visitor interface for mapping between QObject and qapi
has formerly been documented only by reading source code,
making it difficult to propose changes to either
scripts/qapi*.py or to clients without knowing whether those
changes would be safe.  This tries to add documentation,
including mentioning when parameters can be NULL, and where
there are still some interface warts that would be nice
to remove.

Signed-off-by: Eric Blake <address@hidden>
---
 include/qapi/visitor-impl.h |  39 ++++++++-
 include/qapi/visitor.h      | 192 +++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 218 insertions(+), 13 deletions(-)

diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
index df70dd9..7e8f728 100644
--- a/include/qapi/visitor-impl.h
+++ b/include/qapi/visitor-impl.h
@@ -15,48 +15,79 @@
 #include "qapi/error.h"
 #include "qapi/visitor.h"

+/* This file describes the callback interface for implementing a
+ * QObject 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.  Semantics for the
+ * callbacks are generally similar to the counterpart public
+ * interface.  */
+
 struct Visitor
 {
-    /* Must be set */
+    /* Must be provided to visit structs (the string visitors do not
+     * currently visit structs). */
     void (*start_struct)(Visitor *v, void **obj, const char *kind,
                          const char *name, size_t size, Error **errp);
+    /* Must be provided if start_struct is present. */
     void (*end_struct)(Visitor *v, Error **errp);

+    /* May be NULL; most useful for input visitors. */
     void (*start_implicit_struct)(Visitor *v, void **obj, size_t size,
                                   Error **errp);
+    /* May be NULL */
     void (*end_implicit_struct)(Visitor *v, Error **errp);

+    /* Must be set */
     void (*start_list)(Visitor *v, const char *name, Error **errp);
+    /* Must be set */
     GenericList *(*next_list)(Visitor *v, GenericList **list, Error **errp);
+    /* Must be set */
     void (*end_list)(Visitor *v, Error **errp);

+    /* Must be set, although the helpers input_type_enum() and
+     * output_type_enum() can be used.  */
     void (*type_enum)(Visitor *v, int *obj, const char * const strings[],
                       const char *kind, const char *name, Error **errp);
+    /* May be NULL; most useful for input visitors. */
     void (*get_next_type)(Visitor *v, qtype_code *type, bool promote_int,
                           const char *name, Error **errp);

+    /* Must be set */
     void (*type_int)(Visitor *v, int64_t *obj, const char *name, Error **errp);
+    /* Must be set */
     void (*type_bool)(Visitor *v, bool *obj, const char *name, Error **errp);
+    /* Must be set */
     void (*type_str)(Visitor *v, char **obj, const char *name, Error **errp);
+
+    /* Must be provided to visit numbers (the opts visitor does not
+     * currently visit non-integers). */
     void (*type_number)(Visitor *v, double *obj, const char *name,
                         Error **errp);
+    /* Must be provided to visit arbitrary QTypes (the opts and string
+     * visitors do not currently visit arbitrary types).  */
     void (*type_any)(Visitor *v, QObject **obj, const char *name,
                      Error **errp);

-    /* May be NULL */
+    /* May be NULL; most useful for input visitors. */
     void (*optional)(Visitor *v, bool *present, const char *name,
                      Error **errp);

-    /* visit_type_uint64() falls back to (*type_int)() as needed */
+    /* Only required to visit uint64 differently than (*type_int)().  */
     void (*type_uint64)(Visitor *v, uint64_t *obj, const char *name,
                         Error **errp);
-    /* visit_type_size() falls back to (*type_uint64)() as needed */
+    /* Only required to visit sizes differently than (*type_uint64)().  */
     void (*type_size)(Visitor *v, uint64_t *obj, const char *name,
                       Error **errp);
 };

+/**
+ * A generic visitor.type_enum suitable for input visitors.
+ */
 void input_type_enum(Visitor *v, int *obj, const char * const strings[],
                      const char *kind, const char *name, Error **errp);
+/**
+ * A generic visitor.type_enum suitable for output visitors.
+ */
 void output_type_enum(Visitor *v, int *obj, const char * const strings[],
                       const char *kind, const char *name, Error **errp);

diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index 5436069..4ac0c3a 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -18,6 +18,17 @@
 #include "qapi/error.h"
 #include <stdlib.h>

+/* This file describes the client view for visiting a map between
+ * QObjects and another representation (command line options, strings,
+ * or generated QAPI C structs).  An input visitor converts from
+ * QObject to another form; an output visitor converts from the other
+ * form back into QObjects.  These functions seldom need to be called
+ * directly, but are instead used by code generated by
+ * scripts/qapi-visit.py.  For the visitor callback contracts, see
+ * visitor-impl.h. */
+
+/* This struct is layout-compatible with all other *List structs
+ * created by the qapi generator. */
 typedef struct GenericList
 {
     union {
@@ -27,34 +38,197 @@ typedef struct GenericList
     struct GenericList *next;
 } GenericList;

+/**
+ * Prepare to visit a QDict with C type @kind tied to QDict key @name.
+ * @name will be NULL if this is visited as part of a QList.
+ * The caller then makes a series of visit calls for each key expected
+ * in the dictionary, followed by a call to visit_end_struct(). For an
+ * input visitor, @obj can be NULL to validate that the visit will
+ * succeed; otherwise, address@hidden is assigned with an allocation of @size
+ * bytes. For other visitors, address@hidden is the object to visit. Set 
address@hidden
+ * on failure.
+ *
+ * FIXME: address@hidden can be modified even on error; this can lead to
+ * memory leaks if clients aren't careful.
+ */
 void visit_start_struct(Visitor *v, void **obj, const char *kind,
                         const char *name, size_t size, Error **errp);
+/**
+ * Complete a struct started earlier.
+ * Must be called after any successful use of visit_start_struct(),
+ * even if intermediate processing was skipped due to errors.
+ */
 void visit_end_struct(Visitor *v, Error **errp);
+
+/**
+ * Prepare to visit an implicit struct.
+ * Similar to visit_start_struct(), except that this will visit a
+ * C pointer pointing to @size bytes, and where the QDict fields are
+ * part of the parent object.
+ *
+ * FIXME: address@hidden can be modified even on error; this can lead to
+ * memory leaks if clients aren't careful.
+ */
 void visit_start_implicit_struct(Visitor *v, void **obj, size_t size,
                                  Error **errp);
+/**
+ * Complete an implicit struct started earlier.
+ * Must be called after any successful use of visit_start_implicit_struct(),
+ * even if intermediate processing was skipped due to errors.
+ */
 void visit_end_implicit_struct(Visitor *v, Error **errp);
+
+/**
+ * Prepare to visit a QList tied to QDict key @name.
+ * @name will be NULL if this is visited as part of a QList.
+ * After calling this, the elements must be collected until
+ * visit_next_list() returns NULL, then visit_end_list() must be
+ * used to complete the visit.
+ */
 void visit_start_list(Visitor *v, const char *name, Error **errp);
+/**
+ * Collect the next list member and append it to address@hidden
+ * Start with address@hidden of NULL, then subsequent iterations should pass
+ * address@hidden pointing to the previous return 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.
+ */
 GenericList *visit_next_list(Visitor *v, GenericList **list, Error **errp);
+/**
+ * Complete the list started earlier.
+ * Must be called after any successful use of visit_start_list(),
+ * even if intermediate processing was skipped due to errors.
+ */
 void visit_end_list(Visitor *v, Error **errp);
+
+/**
+ * Check if an optional member @name of a QDict needs visiting.
+ * For input visitors, set address@hidden according to whether the
+ * corresponding visit_type_*() needs calling; for other visitors,
+ * leave address@hidden unchanged.
+ */
 void visit_optional(Visitor *v, bool *present, const char *name,
                     Error **errp);
+
+/**
+ * Determine the qtype of the item @name in the current QDict visit.
+ * For input visitors, set address@hidden to the correct qtype of a qapi
+ * alternate type; for other visitors, leave address@hidden unchanged.
+ * If @promote_int, treat integers as numbers.
+ */
 void visit_get_next_type(Visitor *v, qtype_code *type, bool promote_int,
                          const char *name, Error **errp);
+
+/**
+ * Visit an enum value tied to @name in the current QDict visit.
+ * @name will be NULL if this is visited as part of a QList.
+ * For input visitors, parse a string and set address@hidden to the numeric 
value
+ * of the enum type @kind using @strings as the mapping; for output
+ * visitors, reverse the mapping and visit the output string determined
+ * by address@hidden
+ */
 void visit_type_enum(Visitor *v, int *obj, const char * const strings[],
                      const char *kind, const char *name, Error **errp);
+
+/**
+ * Visit an integer value tied to @name in the current QDict visit.
+ * @name will be NULL if this is visited as part of a QList.
+ * For input visitors, set address@hidden to the parsed value; for other 
visitors,
+ * leave address@hidden unchanged.
+ */
 void visit_type_int(Visitor *v, int64_t *obj, const char *name, Error **errp);
-void visit_type_uint8(Visitor *v, uint8_t *obj, const char *name, Error 
**errp);
-void visit_type_uint16(Visitor *v, uint16_t *obj, const char *name, Error 
**errp);
-void visit_type_uint32(Visitor *v, uint32_t *obj, const char *name, Error 
**errp);
-void visit_type_uint64(Visitor *v, uint64_t *obj, const char *name, Error 
**errp);
+/**
+ * Visit a uint8_t value tied to @name in the current QDict visit.
+ * Like visit_type_int(), except clamps the value to uint8_t range.
+ */
+void visit_type_uint8(Visitor *v, uint8_t *obj, const char *name,
+                      Error **errp);
+/**
+ * Visit a uint16_t value tied to @name in the current QDict visit.
+ * Like visit_type_int(), except clamps the value to uint16_t range.
+ */
+void visit_type_uint16(Visitor *v, uint16_t *obj, const char *name,
+                       Error **errp);
+/**
+ * Visit a uint32_t value tied to @name in the current QDict visit.
+ * Like visit_type_int(), except clamps the value to uint32_t range.
+ */
+void visit_type_uint32(Visitor *v, uint32_t *obj, const char *name,
+                       Error **errp);
+/**
+ * Visit a uint64_t value tied to @name in the current QDict visit.
+ * Like visit_type_int(), except clamps the value to uint64_t range
+ * (that is, ensures it is unsigned).
+ */
+void visit_type_uint64(Visitor *v, uint64_t *obj, const char *name,
+                       Error **errp);
+/**
+ * Visit an int8_t value tied to @name in the current QDict visit.
+ * Like visit_type_int(), except clamps the value to int8_t range.
+ */
 void visit_type_int8(Visitor *v, int8_t *obj, const char *name, Error **errp);
-void visit_type_int16(Visitor *v, int16_t *obj, const char *name, Error 
**errp);
-void visit_type_int32(Visitor *v, int32_t *obj, const char *name, Error 
**errp);
-void visit_type_int64(Visitor *v, int64_t *obj, const char *name, Error 
**errp);
-void visit_type_size(Visitor *v, uint64_t *obj, const char *name, Error 
**errp);
+/**
+ * Visit an int16_t value tied to @name in the current QDict visit.
+ * Like visit_type_int(), except clamps the value to int16_t range.
+ */
+void visit_type_int16(Visitor *v, int16_t *obj, const char *name,
+                      Error **errp);
+/**
+ * Visit an uint32_t value tied to @name in the current QDict visit.
+ * Like visit_type_int(), except clamps the value to int32_t range.
+ */
+void visit_type_int32(Visitor *v, int32_t *obj, const char *name,
+                      Error **errp);
+/**
+ * Visit an int64_t value tied to @name in the current QDict visit.
+ * Like visit_type_int(), except clamps the value to int64_t range.
+ */
+void visit_type_int64(Visitor *v, int64_t *obj, const char *name,
+                      Error **errp);
+/**
+ * Visit a uint64_t value tied to @name in the current QDict visit.
+ * Like visit_type_uint64(), except that some visitors may choose to
+ * recognize additional suffixes for easily scaling input values.
+ */
+void visit_type_size(Visitor *v, uint64_t *obj, const char *name,
+                     Error **errp);
+
+/**
+ * Visit a boolean value tied to @name in the current QDict visit.
+ * @name will be NULL if this is visited as part of a QList.
+ * Input visitors set address@hidden to the value; other visitors will leave
+ * address@hidden unchanged.
+ */
 void visit_type_bool(Visitor *v, bool *obj, const char *name, Error **errp);
+
+/**
+ * Visit a string value tied to @name in the current QDict visit.
+ * @name will be NULL if this is visited as part of a QList.
+ * @obj must be non-NULL.  Input visitors set address@hidden to the parsed 
string;
+ * while output visitors leave address@hidden unchanged, except that a NULL 
address@hidden
+ * must be treated the same as "".
+ *
+ * FIXME: Unfortunately not const-correct for output visitors.
+ */
 void visit_type_str(Visitor *v, char **obj, const char *name, Error **errp);
-void visit_type_number(Visitor *v, double *obj, const char *name, Error 
**errp);
+
+/**
+ * Visit a number value tied to @name in the current QDict visit.
+ * @name will be NULL if this is visited as part of a QList.
+ * Input visitors set address@hidden to the value; other visitors will leave
+ * address@hidden unchanged.
+ */
+void visit_type_number(Visitor *v, double *obj, const char *name,
+                       Error **errp);
+
+/**
+ * Visit an arbitrary qtype value tied to @name in the current QDict visit.
+ * @name will be NULL if this is visited as part of a QList.
+ * Input visitors set address@hidden to the value; other visitors will leave
+ * address@hidden unchanged.
+ */
 void visit_type_any(Visitor *v, QObject **obj, const char *name, Error **errp);

 #endif
-- 
2.4.3