[Qemu-devel] [PATCH v10 13/25] qapi-visit: Convert to new qapi union layout

[Eric Blake]

We have two issues with our qapi union layout:
1) Even though the QMP wire format spells the tag 'type', the
C code spells it 'kind', requiring some hacks in the generator.
2) The C struct uses an anonymous union, which places all tag
values in the same namespace as all non-variant members. This
leads to spurious collisions if a tag value matches a QMP name.

Make the conversion to the new layout for qapi-visit.py.  Also
make a change in qapi.py to quit using the name 'kind', and
the corresponding change in the testsuite.  However, many more
changes will be made to the testsuite later, including the
entire deletion of union-clash-type.json, after the completion
of the conversion eliminates collisions between union tag
branch names and non-variant names.

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

---
v10: new patch, split from 7/17 an 8/17
v9: new patch, but incorporates parts of v5 31/46 and Markus' RFC:
http://lists.gnu.org/archive/html/qemu-devel/2015-10/msg02236.html
---
 scripts/qapi-visit.py                  | 24 +++++++++---------------
 scripts/qapi.py                        |  2 +-
 tests/qapi-schema/union-clash-type.err |  2 +-
 3 files changed, 11 insertions(+), 17 deletions(-)

diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 3c23c53..421fcb6 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -187,18 +187,18 @@ 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)->kind, %(c_name)s_qtypes, name, 
&err);
+    visit_get_next_type(v, (int*) &(*obj)->type, %(c_name)s_qtypes, name, 
&err);
     if (err) {
         goto out_obj;
     }
-    switch ((*obj)->kind) {
+    switch ((*obj)->type) {
 ''',
                 c_name=c_name(name))

     for var in variants.variants:
         ret += mcgen('''
     case %(case)s:
-        visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, name, &err);
+        visit_type_%(c_type)s(v, &(*obj)->u.%(c_name)s, name, &err);
         break;
 ''',
                      case=c_enum_const(variants.tag_member.type.name,
@@ -259,22 +259,16 @@ 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)s", &err);
 ''',
                      c_type=variants.tag_member.type.c_name(),
-                     # TODO ugly special case for simple union
-                     # Use same tag name in C as on the wire to get rid of
-                     # it, then: c_name=c_name(variants.tag_member.name)
-                     c_name='kind',
+                     c_name=c_name(variants.tag_member.name),
                      name=variants.tag_member.name)
     ret += gen_err_check(label='out_obj')
     ret += mcgen('''
-    if (!visit_start_union(v, !!(*obj)->data, &err) || err) {
+    if (!visit_start_union(v, !!(*obj)->u.data, &err) || err) {
         goto out_obj;
     }
     switch ((*obj)->%(c_name)s) {
 ''',
-                 # TODO ugly special case for simple union
-                 # Use same tag name in C as on the wire to get rid of
-                 # it, then: c_name=c_name(variants.tag_member.name)
-                 c_name=c_name(variants.tag_name or 'kind'))
+                 c_name=c_name(variants.tag_member.name))

     for var in variants.variants:
         # TODO ugly special case for simple union
@@ -286,13 +280,13 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, 
const char *name, Error
                                        var.name))
         if simple_union_type:
             ret += mcgen('''
-        visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, "data", &err);
+        visit_type_%(c_type)s(v, &(*obj)->u.%(c_name)s, "data", &err);
 ''',
                          c_type=simple_union_type.c_name(),
                          c_name=c_name(var.name))
         else:
             ret += mcgen('''
-        visit_type_implicit_%(c_type)s(v, &(*obj)->%(c_name)s, &err);
+        visit_type_implicit_%(c_type)s(v, &(*obj)->u.%(c_name)s, &err);
 ''',
                          c_type=var.type.c_name(),
                          c_name=c_name(var.name))
@@ -308,7 +302,7 @@ out_obj:
     error_propagate(errp, err);
     err = NULL;
     if (*obj) {
-        visit_end_union(v, !!(*obj)->data, &err);
+        visit_end_union(v, !!(*obj)->u.data, &err);
     }
     error_propagate(errp, err);
     err = NULL;
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 04bcbf7..8b9f5f7 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -548,7 +548,7 @@ def check_union(expr, expr_info):
     base = expr.get('base')
     discriminator = expr.get('discriminator')
     members = expr['data']
-    values = {'MAX': '(automatic)', 'KIND': '(automatic)'}
+    values = {'MAX': '(automatic)', 'TYPE': '(automatic tag)'}

     # Two types of unions, determined by discriminator.

diff --git a/tests/qapi-schema/union-clash-type.err 
b/tests/qapi-schema/union-clash-type.err
index a5dead1..eca7c1d 100644
--- a/tests/qapi-schema/union-clash-type.err
+++ b/tests/qapi-schema/union-clash-type.err
@@ -1 +1 @@
-tests/qapi-schema/union-clash-type.json:8: Union 'TestUnion' member 'kind' 
clashes with '(automatic)'
+tests/qapi-schema/union-clash-type.json:8: Union 'TestUnion' member 'type' 
clashes with '(automatic tag)'
-- 
2.4.3