[Qemu-devel] [PATCH v6 08/15] qapi: Simplify visiting of alternate types

[Eric Blake]

Previously, working with alternates required two enums, and
some indirection: for type Foo, we created Foo_qtypes[] which
maps each qtype to a member of FooKind_lookup[], then use
FooKind_lookup[] like we do for other union types.

This has a subtle bug: since the values of FooKind_lookup
start at zero, all entries of Foo_qtypes that were not
explicitly initialized map to the same branch of the union as
the first member of the alternate, rather than triggering a
failure in visit_get_next_type().  Fortunately, the bug
seldom bites; the very next thing the input visitor does is
try to parse the incoming JSON with the wrong parser, which
fails; the output visitor is not used with a C struct in that
state, and the dealloc visitor has nothing to clean up (so
there is no leak).

However, it IS observable in one case: the behavior of an
alternate that contains a 'number' member but no 'int' member
differs according to whether the 'number' was first in the
qapi definition, and when the input being parsed is an integer;
this is because the 'number' parser accepts QTYPE_QINT in
addition to the expected QTYPE_QFLOAT.  A later patch will worry
about fixing alternates to parse all inputs that a non-alternate
'number' would accept, for now it is still marked FIXME.

This patch fixes the validation bug by deleting the indirection,
and modifying get_next_type() to directly return a qtype code.
There is no longer a need to generate an implicit FooKind array
associated with the alternate type (since the QMP wire format
never uses the stringized counterparts of the C union member
names).  Next, the generated visitor is fixed to properly detect
unexpected qtypes in the switch statement.  With a bit of work
to the previously-added QAPISchemaAlternateTypeTag, we can wrap
the layout change so that spots in qapi-types that are shared
with other unions still work.

Callers now have to know the QTYPE_* mapping when looking at the
discriminator; but so far, only the testsuite was even using the
C struct of an alternate types.  If that gets too confusing, we
could reintroduce FooKind, but initialize it differently than
most generated arrays, as in:
  typedef enum FooKind {
      FOO_KIND_A = QTYPE_QDICT,
      FOO_KIND_B = QTYPE_QINT,
  } FooKind;
to create nicer aliases for knowing when to use foo->a or foo->b
when inspecting foo->type.  But without a current client, I
didn't see the point of doing it now.

There is a user-visible side effect to this change, but I
consider it to be an improvement. Previously,
the invalid QMP command:
  {"execute":"blockdev-add", "arguments":{"options":
    {"driver":"raw", "id":"a", "file":true}}}
failed with:
  {"error": {"class": "GenericError",
    "desc": "Invalid parameter type for 'file', expected: QDict"}}
Now it fails with:
  {"error": {"class": "GenericError",
    "desc": "Invalid parameter type for 'file', expected: BlockdevRef"}}

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

---
v6: rebase onto tag_member subclass, testsuite, gen_err_check(),
and info improvements
---
 docs/qapi-code-gen.txt                        |  3 --
 include/qapi/visitor-impl.h                   |  3 +-
 include/qapi/visitor.h                        |  8 +++++-
 qapi/qapi-visit-core.c                        |  4 +--
 qapi/qmp-input-visitor.c                      |  4 +--
 scripts/qapi-types.py                         | 34 -----------------------
 scripts/qapi-visit.py                         | 12 ++++----
 scripts/qapi.py                               | 40 ++++++++++++++++++---------
 tests/qapi-schema/alternate-clash-members.err |  2 +-
 tests/qapi-schema/alternate-empty.out         |  1 -
 tests/qapi-schema/qapi-schema-test.out        |  8 ------
 tests/test-qmp-input-visitor.c                | 33 ++++++++++++----------
 tests/test-qmp-output-visitor.c               | 21 ++++++++++----
 13 files changed, 82 insertions(+), 91 deletions(-)

diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index a2feb8c..0cc432b 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -379,9 +379,6 @@ where each branch of the union names a QAPI type.  For 
example:
    'data': { 'definition': 'BlockdevOptions',
              'reference': 'str' } }

-Just like for a simple union, an implicit C enum 'NameKind' is created
-to enumerate the branches for the alternate 'Name'.
-
 Unlike a union, the discriminator string is never passed on the wire
 for the Client JSON Protocol.  Instead, the value's JSON type serves
 as an implicit discriminator, which in turn means that an alternate
diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
index 8c0ba57..6d95b36 100644
--- a/include/qapi/visitor-impl.h
+++ b/include/qapi/visitor-impl.h
@@ -32,7 +32,8 @@ struct Visitor

     void (*type_enum)(Visitor *v, int *obj, const char * const strings[],
                       const char *kind, const char *name, Error **errp);
-    void (*get_next_type)(Visitor *v, int *kind, const int *qobjects,
+    /* May be NULL; most useful for input visitors. */
+    void (*get_next_type)(Visitor *v, qtype_code *type,
                           const char *name, Error **errp);

     void (*type_int)(Visitor *v, int64_t *obj, const char *name, Error **errp);
diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index cfc19a6..b765993 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -41,7 +41,13 @@ GenericList *visit_next_list(Visitor *v, GenericList **list, 
Error **errp);
 void visit_end_list(Visitor *v, Error **errp);
 void visit_optional(Visitor *v, bool *present, const char *name,
                     Error **errp);
-void visit_get_next_type(Visitor *v, int *obj, const int *qtypes,
+
+/**
+ * Determine the qtype of the item @name in the current object visit.
+ * For input visitors, set address@hidden to the correct qtype of a qapi
+ * alternate type; for other visitors, leave address@hidden unchanged.
+ */
+void visit_get_next_type(Visitor *v, qtype_code *type,
                          const char *name, Error **errp);
 void visit_type_enum(Visitor *v, int *obj, const char * const strings[],
                      const char *kind, const char *name, Error **errp);
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index 59ed506..3f24daa 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -81,11 +81,11 @@ void visit_optional(Visitor *v, bool *present, const char 
*name,
     }
 }

-void visit_get_next_type(Visitor *v, int *obj, const int *qtypes,
+void visit_get_next_type(Visitor *v, qtype_code *type,
                          const char *name, Error **errp)
 {
     if (v->get_next_type) {
-        v->get_next_type(v, obj, qtypes, name, errp);
+        v->get_next_type(v, type, name, errp);
     }
 }

diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
index 5dd9ed5..803ffad 100644
--- a/qapi/qmp-input-visitor.c
+++ b/qapi/qmp-input-visitor.c
@@ -208,7 +208,7 @@ static void qmp_input_end_list(Visitor *v, Error **errp)
     qmp_input_pop(qiv, errp);
 }

-static void qmp_input_get_next_type(Visitor *v, int *kind, const int *qobjects,
+static void qmp_input_get_next_type(Visitor *v, qtype_code *type,
                                     const char *name, Error **errp)
 {
     QmpInputVisitor *qiv = to_qiv(v);
@@ -218,7 +218,7 @@ static void qmp_input_get_next_type(Visitor *v, int *kind, 
const int *qobjects,
         error_setg(errp, QERR_MISSING_PARAMETER, name ? name : "null");
         return;
     }
-    *kind = qobjects[qobject_type(qobj)];
+    *type = qobject_type(qobj);
 }

 static void qmp_input_type_int(Visitor *v, int64_t *obj, const char *name,
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 138920c..884eefd 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -87,38 +87,6 @@ struct %(c_name)s {
     return ret


-def gen_alternate_qtypes_decl(name):
-    return mcgen('''
-
-extern const int %(c_name)s_qtypes[];
-''',
-                 c_name=c_name(name))
-
-
-def gen_alternate_qtypes(name, variants):
-    ret = mcgen('''
-
-const int %(c_name)s_qtypes[QTYPE_MAX] = {
-''',
-                c_name=c_name(name))
-
-    for var in variants.variants:
-        qtype = var.type.alternate_qtype()
-        assert qtype
-
-        ret += mcgen('''
-    [%(qtype)s] = %(enum_const)s,
-''',
-                     qtype=qtype,
-                     enum_const=c_enum_const(variants.tag_member.type.name,
-                                             var.name))
-
-    ret += mcgen('''
-};
-''')
-    return ret
-
-
 def gen_union(name, base, variants):
     ret = mcgen('''

@@ -266,9 +234,7 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor):

     def visit_alternate_type(self, name, info, variants):
         self._fwdecl += gen_fwd_object_or_array(name)
-        self._fwdefn += gen_alternate_qtypes(name, variants)
         self.decl += gen_union(name, None, variants)
-        self.decl += gen_alternate_qtypes_decl(name)
         self._gen_type_cleanup(name)

 # If you link code generated from multiple schemata, you want only one
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index d82e2a3..571bfa3 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -179,7 +179,7 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, 
const char *name, Error
     if (err) {
         goto out;
     }
-    visit_get_next_type(v, (int*) &(*obj)->type, %(c_name)s_qtypes, name, 
&err);
+    visit_get_next_type(v, &(*obj)->type, name, &err);
     if (err) {
         goto out_obj;
     }
@@ -193,14 +193,14 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, 
const char *name, Error
         visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, name, &err);
         break;
 ''',
-                     case=c_enum_const(variants.tag_member.type.name,
-                                       var.name),
+                     case=var.type.alternate_qtype(),
                      c_type=var.type.c_name(),
                      c_name=var.c_name())

     ret += mcgen('''
     default:
-        abort();
+        error_setg(&err, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
+                   "%(name)s");
     }
 out_obj:
     error_propagate(errp, err);
@@ -209,7 +209,8 @@ out_obj:
 out:
     error_propagate(errp, err);
 }
-''')
+''',
+                 name=name)

     return ret

@@ -411,6 +412,7 @@ fdef.write(mcgen('''

 fdecl.write(mcgen('''
 #include "qapi/visitor.h"
+#include "qapi/qmp/qerror.h"
 #include "%(prefix)sqapi-types.h"

 ''',
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 9a734a8..430c4bc 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -868,10 +868,7 @@ class QAPISchemaEnumType(QAPISchemaType):
                 if self.is_implicit():
                     owner = schema.lookup_type(self.name[:-4])
                     assert owner
-                    if isinstance(owner, QAPISchemaAlternateType):
-                        description = "Alternate '%s' branch" % owner.name
-                    else:
-                        description = "Union '%s' branch" % owner.name
+                    description = "Union '%s' branch" % owner.name
                 else:
                     description = "Enum '%s' value" % self.name
                 raise QAPIExprError(self.info,
@@ -1060,7 +1057,8 @@ class QAPISchemaObjectTypeVariants(object):
             assert self.tag_name == self.tag_member.name
         else:
             self.tag_member.check(schema, info, members, seen)
-        assert isinstance(self.tag_member.type, QAPISchemaEnumType)
+        if not isinstance(self.tag_member, QAPISchemaAlternateTypeTag):
+            assert isinstance(self.tag_member.type, QAPISchemaEnumType)
         for v in self.variants:
             vseen = dict(seen)
             v.check(schema, info, self.tag_member.type, vseen,
@@ -1073,7 +1071,8 @@ class 
QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember):

     def check(self, schema, info, tag_type, seen, flat):
         QAPISchemaObjectTypeMember.check(self, schema, info, [], seen)
-        assert self.name in tag_type.values
+        if tag_type:
+            assert self.name in tag_type.values
         if flat:
             # For flat unions, check that each QMP member does not
             # collide with any non-variant members. No type can
@@ -1113,7 +1112,16 @@ class QAPISchemaAlternateType(QAPISchemaType):
         self.variants = variants

     def check(self, schema):
-        self.variants.check(schema, self.info, [], {})
+        seen = {}
+        self.variants.check(schema, self.info, [], seen)
+        # Ensure no branch names collide with non-variant members
+        for v in self.variants.variants:
+            if v.c_name() in seen:
+                raise QAPIExprError(self.info,
+                                    "%s collides with %s"
+                                    % (v.describe(),
+                                       seen[v.c_name()].describe()))
+            seen[v.c_name()] = v

     def json_type(self):
         return 'value'
@@ -1123,12 +1131,21 @@ class QAPISchemaAlternateType(QAPISchemaType):


 class QAPISchemaAlternateTypeTag(QAPISchemaObjectTypeMember):
-    def __init__(self, enum_type):
-        QAPISchemaObjectTypeMember.__init__(self, 'type', enum_type, False)
+    def __init__(self):
+        QAPISchemaObjectTypeMember.__init__(self, 'type', '', False)
+
+    def check(self, schema, info, all_members, seen):
+        assert self.owner
+        assert len(seen) == 0
+        all_members.append(self)
+        seen[self.c_name()] = self

     def _describe(self):
         return 'implicit tag'

+    def c_type(self):
+        return 'qtype_code'
+

 class QAPISchemaCommand(QAPISchemaEntity):
     def __init__(self, name, info, arg_type, ret_type, gen, success_response):
@@ -1323,10 +1340,7 @@ class QAPISchema(object):
         data = expr['data']
         variants = [self._make_variant(key, value)
                     for (key, value) in data.iteritems()]
-        # TODO simplify this to avoid the need for an implicit enum
-        tag_member = QAPISchemaAlternateTypeTag(
-            self._make_implicit_enum_type(name, info,
-                                          [v.name for v in variants]))
+        tag_member = QAPISchemaAlternateTypeTag()
         self._def_entity(
             QAPISchemaAlternateType(name, info,
                                     QAPISchemaObjectTypeVariants(None,
diff --git a/tests/qapi-schema/alternate-clash-members.err 
b/tests/qapi-schema/alternate-clash-members.err
index 0adf737..ad8720e 100644
--- a/tests/qapi-schema/alternate-clash-members.err
+++ b/tests/qapi-schema/alternate-clash-members.err
@@ -1 +1 @@
-tests/qapi-schema/alternate-clash-members.json:7: Alternate 'Alt1' branch 
'a_b' clashes with 'a-b'
+tests/qapi-schema/alternate-clash-members.json:7: 'a_b' (branch of Alt1) 
collides with 'a-b' (branch of Alt1)
diff --git a/tests/qapi-schema/alternate-empty.out 
b/tests/qapi-schema/alternate-empty.out
index 0f153b6..9b010d8 100644
--- a/tests/qapi-schema/alternate-empty.out
+++ b/tests/qapi-schema/alternate-empty.out
@@ -1,4 +1,3 @@
 object :empty
 alternate Alt
     case i: int
-enum AltKind ['i']
diff --git a/tests/qapi-schema/qapi-schema-test.out 
b/tests/qapi-schema/qapi-schema-test.out
index d452c5b..6826fb6 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -56,27 +56,21 @@ object :obj-user_def_cmd2 arguments
 alternate AltIntNum
     case i: int
     case n: number
-enum AltIntNumKind ['i', 'n']
 alternate AltNumInt
     case n: number
     case i: int
-enum AltNumIntKind ['n', 'i']
 alternate AltNumStr
     case n: number
     case s: str
-enum AltNumStrKind ['n', 's']
 alternate AltStrBool
     case s: str
     case b: bool
-enum AltStrBoolKind ['s', 'b']
 alternate AltStrInt
     case s: str
     case i: int
-enum AltStrIntKind ['s', 'i']
 alternate AltStrNum
     case s: str
     case n: number
-enum AltStrNumKind ['s', 'n']
 event EVENT_A None
 event EVENT_B None
 event EVENT_C :obj-EVENT_C data
@@ -109,7 +103,6 @@ alternate UserDefAlternate
     case uda: UserDefA
     case s: str
     case i: int
-enum UserDefAlternateKind ['uda', 's', 'i']
 object UserDefB
     member intb: int optional=False
     member a-b: bool optional=True
@@ -174,7 +167,6 @@ event __ORG.QEMU_X-EVENT __org.qemu_x-Struct
 alternate __org.qemu_x-Alt
     case __org.qemu_x-branch: str
     case b: __org.qemu_x-Base
-enum __org.qemu_x-AltKind ['__org.qemu_x-branch', 'b']
 object __org.qemu_x-Base
     member __org.qemu_x-member1: __org.qemu_x-Enum optional=False
 enum __org.qemu_x-Enum ['__org.qemu_x-value']
diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c
index 9540fa0..c5ed19a 100644
--- a/tests/test-qmp-input-visitor.c
+++ b/tests/test-qmp-input-visitor.c
@@ -338,14 +338,14 @@ static void 
test_visitor_in_alternate(TestInputVisitorData *data,

     v = visitor_input_test_init(data, "42");
     visit_type_UserDefAlternate(v, &tmp, NULL, &error_abort);
-    g_assert_cmpint(tmp->type, ==, USER_DEF_ALTERNATE_KIND_I);
+    g_assert_cmpint(tmp->type, ==, QTYPE_QINT);
     g_assert_cmpint(tmp->i, ==, 42);
     qapi_free_UserDefAlternate(tmp);
     visitor_input_teardown(data, NULL);

     v = visitor_input_test_init(data, "'string'");
     visit_type_UserDefAlternate(v, &tmp, NULL, &error_abort);
-    g_assert_cmpint(tmp->type, ==, USER_DEF_ALTERNATE_KIND_S);
+    g_assert_cmpint(tmp->type, ==, QTYPE_QSTRING);
     g_assert_cmpstr(tmp->s, ==, "string");
     qapi_free_UserDefAlternate(tmp);
     visitor_input_teardown(data, NULL);
@@ -381,11 +381,10 @@ static void 
test_visitor_in_alternate_number(TestInputVisitorData *data,
     qapi_free_AltStrBool(asb);
     visitor_input_teardown(data, NULL);

-    /* FIXME: Order of alternate should not affect semantics; asn should
-     * parse the same as ans */
+    /* FIXME: integer should parse as number */
     v = visitor_input_test_init(data, "42");
     visit_type_AltStrNum(v, &asn, NULL, &err);
-    /* FIXME g_assert_cmpint(asn->type, == ALT_STR_NUM_KIND_N); */
+    /* FIXME g_assert_cmpint(asn->type, ==, QTYPE_QFLOAT); */
     /* FIXME g_assert_cmpfloat(asn->n, ==, 42); */
     g_assert(err);
     error_free(err);
@@ -393,30 +392,34 @@ static void 
test_visitor_in_alternate_number(TestInputVisitorData *data,
     qapi_free_AltStrNum(asn);
     visitor_input_teardown(data, NULL);

+    /* FIXME: integer should parse as number */
     v = visitor_input_test_init(data, "42");
-    visit_type_AltNumStr(v, &ans, NULL, &error_abort);
-    g_assert_cmpint(ans->type, ==, ALT_NUM_STR_KIND_N);
-    g_assert_cmpfloat(ans->n, ==, 42);
+    visit_type_AltNumStr(v, &ans, NULL, &err);
+    /* FIXME g_assert_cmpint(ans->type, ==, QTYPE_QFLOAT); */
+    /* FIXME g_assert_cmpfloat(ans->n, ==, 42); */
+    g_assert(err);
+    error_free(err);
+    err = NULL;
     qapi_free_AltNumStr(ans);
     visitor_input_teardown(data, NULL);

     v = visitor_input_test_init(data, "42");
     visit_type_AltStrInt(v, &asi, NULL, &error_abort);
-    g_assert_cmpint(asi->type, ==, ALT_STR_INT_KIND_I);
+    g_assert_cmpint(asi->type, ==, QTYPE_QINT);
     g_assert_cmpint(asi->i, ==, 42);
     qapi_free_AltStrInt(asi);
     visitor_input_teardown(data, NULL);

     v = visitor_input_test_init(data, "42");
     visit_type_AltIntNum(v, &ain, NULL, &error_abort);
-    g_assert_cmpint(ain->type, ==, ALT_INT_NUM_KIND_I);
+    g_assert_cmpint(ain->type, ==, QTYPE_QINT);
     g_assert_cmpint(ain->i, ==, 42);
     qapi_free_AltIntNum(ain);
     visitor_input_teardown(data, NULL);

     v = visitor_input_test_init(data, "42");
     visit_type_AltNumInt(v, &ani, NULL, &error_abort);
-    g_assert_cmpint(ani->type, ==, ALT_NUM_INT_KIND_I);
+    g_assert_cmpint(ani->type, ==, QTYPE_QINT);
     g_assert_cmpint(ani->i, ==, 42);
     qapi_free_AltNumInt(ani);
     visitor_input_teardown(data, NULL);
@@ -433,14 +436,14 @@ static void 
test_visitor_in_alternate_number(TestInputVisitorData *data,

     v = visitor_input_test_init(data, "42.5");
     visit_type_AltStrNum(v, &asn, NULL, &error_abort);
-    g_assert_cmpint(asn->type, ==, ALT_STR_NUM_KIND_N);
+    g_assert_cmpint(asn->type, ==, QTYPE_QFLOAT);
     g_assert_cmpfloat(asn->n, ==, 42.5);
     qapi_free_AltStrNum(asn);
     visitor_input_teardown(data, NULL);

     v = visitor_input_test_init(data, "42.5");
     visit_type_AltNumStr(v, &ans, NULL, &error_abort);
-    g_assert_cmpint(ans->type, ==, ALT_NUM_STR_KIND_N);
+    g_assert_cmpint(ans->type, ==, QTYPE_QFLOAT);
     g_assert_cmpfloat(ans->n, ==, 42.5);
     qapi_free_AltNumStr(ans);
     visitor_input_teardown(data, NULL);
@@ -455,14 +458,14 @@ static void 
test_visitor_in_alternate_number(TestInputVisitorData *data,

     v = visitor_input_test_init(data, "42.5");
     visit_type_AltIntNum(v, &ain, NULL, &error_abort);
-    g_assert_cmpint(ain->type, ==, ALT_INT_NUM_KIND_N);
+    g_assert_cmpint(ain->type, ==, QTYPE_QFLOAT);
     g_assert_cmpfloat(ain->n, ==, 42.5);
     qapi_free_AltIntNum(ain);
     visitor_input_teardown(data, NULL);

     v = visitor_input_test_init(data, "42.5");
     visit_type_AltNumInt(v, &ani, NULL, &error_abort);
-    g_assert_cmpint(ani->type, ==, ALT_NUM_INT_KIND_N);
+    g_assert_cmpint(ani->type, ==, QTYPE_QFLOAT);
     g_assert_cmpint(ani->n, ==, 42.5);
     qapi_free_AltNumInt(ani);
     visitor_input_teardown(data, NULL);
diff --git a/tests/test-qmp-output-visitor.c b/tests/test-qmp-output-visitor.c
index a710557..e72b22c 100644
--- a/tests/test-qmp-output-visitor.c
+++ b/tests/test-qmp-output-visitor.c
@@ -456,20 +456,31 @@ static void 
test_visitor_out_alternate(TestOutputVisitorData *data,
                                        const void *unused)
 {
     QObject *arg;
-    Error *err = NULL;
+    UserDefAlternate *tmp;

-    UserDefAlternate *tmp = g_malloc0(sizeof(UserDefAlternate));
-    tmp->type = USER_DEF_ALTERNATE_KIND_I;
+    tmp = g_malloc0(sizeof(UserDefAlternate));
+    tmp->type = QTYPE_QINT;
     tmp->i = 42;

-    visit_type_UserDefAlternate(data->ov, &tmp, NULL, &err);
-    g_assert(err == NULL);
+    visit_type_UserDefAlternate(data->ov, &tmp, NULL, &error_abort);
     arg = qmp_output_get_qobject(data->qov);

     g_assert(qobject_type(arg) == QTYPE_QINT);
     g_assert_cmpint(qint_get_int(qobject_to_qint(arg)), ==, 42);

     qapi_free_UserDefAlternate(tmp);
+
+    tmp = g_malloc0(sizeof(UserDefAlternate));
+    tmp->type = QTYPE_QSTRING;
+    tmp->s = g_strdup("hello");
+
+    visit_type_UserDefAlternate(data->ov, &tmp, NULL, &error_abort);
+    arg = qmp_output_get_qobject(data->qov);
+
+    g_assert(qobject_type(arg) == QTYPE_QSTRING);
+    g_assert_cmpstr(qstring_get_str(qobject_to_qstring(arg)), ==, "hello");
+
+    qapi_free_UserDefAlternate(tmp);
 }

 static void test_visitor_out_empty(TestOutputVisitorData *data,
-- 
2.4.3