qapi-visit: Add visitor.type classification

We have three classes of QAPI visitors: input, output, and dealloc.
Currently, all implementations of these visitors have one thing in
common based on their visitor type: the implementation used for the
visit_type_enum() callback.  But since we plan to add more such
common behavior, in relation to documenting and further refining
the semantics, it makes more sense to have the visitor
implementations advertise which class they belong to, so the common
qapi-visit-core code can use that information in multiple places.

A later patch will better document the types of visitors directly
in visitor.h.

For this patch, knowing the class of a visitor implementation lets
us make input_type_enum() and output_type_enum() become static
functions, by replacing the callback function Visitor.type_enum()
with the simpler enum member Visitor.type.  Share a common
assertion in qapi-visit-core as part of the refactoring.

Move comments in opts-visitor.c to match the refactored layout.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1461879932-9020-2-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>

9 files changed, 52 insertions, 42 deletions

diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
index 2bd8f292b2..51c338a43d 100644
--- a/include/qapi/visitor-impl.h
+++ b/include/qapi/visitor-impl.h
@@ -14,6 +14,17 @@
 
 #include "qapi/visitor.h"
 
+/*
+ * 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 */
@@ -36,10 +47,6 @@ struct 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. */
     void (*type_int64)(Visitor *v, const char *name, int64_t *obj,
                        Error **errp);
     /* Must be set. */
@@ -58,11 +65,9 @@ struct Visitor
 
     /* May be NULL; most useful for input visitors. */
     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..690589f37d 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -78,8 +78,19 @@ void visit_end_alternate(Visitor *v);
  */
 bool visit_optional(Visitor *v, const char *name, bool *present);
 
+/*
+ * Visit an enum value.
+ *
+ * @strings expresses the mapping between C enum values and QAPI enum
+ * names; 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);
+
 void visit_type_int(Visitor *v, const char *name, int64_t *obj, Error **errp);
 void visit_type_uint8(Visitor *v, const char *name, uint8_t *obj,
                       Error **errp);
diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
index 602f2609cc..66aeaed87e 100644
--- a/qapi/opts-visitor.c
+++ b/qapi/opts-visitor.c
@@ -317,6 +317,11 @@ opts_type_str(Visitor *v, const char *name, char **obj, Error **errp)
         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,6 +512,8 @@ 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.end_struct   = &opts_end_struct;
 
@@ -514,16 +521,6 @@ opts_visitor_new(const QemuOpts *opts)
     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;
diff --git a/qapi/qapi-dealloc-visitor.c b/qapi/qapi-dealloc-visitor.c
index 69221794ec..c19a459391 100644
--- a/qapi/qapi-dealloc-visitor.c
+++ b/qapi/qapi-dealloc-visitor.c
@@ -163,11 +163,6 @@ 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)
-{
-}
-
 Visitor *qapi_dealloc_get_visitor(QapiDeallocVisitor *v)
 {
     return &v->visitor;
@@ -184,6 +179,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,7 +187,6 @@ 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;
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index fa680c9991..3cd7edcc12 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -72,12 +72,6 @@ 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)
-{
-    v->type_enum(v, name, obj, strings, errp);
-}
-
 void visit_type_int(Visitor *v, const char *name, int64_t *obj, Error **errp)
 {
     v->type_int64(v, name, obj, errp);
@@ -208,14 +202,13 @@ void visit_type_any(Visitor *v, const char *name, QObject **obj, Error **errp)
     v->type_any(v, name, obj, 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 +219,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 +248,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(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);
+    }
+}
diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
index 7cd1b777a0..02d4233463 100644
--- a/qapi/qmp-input-visitor.c
+++ b/qapi/qmp-input-visitor.c
@@ -339,13 +339,13 @@ QmpInputVisitor *qmp_input_visitor_new(QObject *obj)
 
     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.start_list = qmp_input_start_list;
     v->visitor.next_list = qmp_input_next_list;
     v->visitor.end_list = qmp_input_end_list;
     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;
diff --git a/qapi/qmp-output-visitor.c b/qapi/qmp-output-visitor.c
index d44c676317..1f2a7ba646 100644
--- a/qapi/qmp-output-visitor.c
+++ b/qapi/qmp-output-visitor.c
@@ -234,12 +234,12 @@ 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;
diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
index 5ea2d77b5a..97009a6dc7 100644
--- a/qapi/string-input-visitor.c
+++ b/qapi/string-input-visitor.c
@@ -348,7 +348,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;
diff --git a/qapi/string-output-visitor.c b/qapi/string-output-visitor.c
index c2e5c5b92b..0d44d7e6a5 100644
--- a/qapi/string-output-visitor.c
+++ b/qapi/string-output-visitor.c
@@ -351,7 +351,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;