[Qemu-devel] [PATCH v6 19/23] qapi-visit: Unify struct and union visit

[Eric Blake <eblake@redhat.com>]

We are finally at the point where gen_visit_struct() and
gen_visit_union() can be unified to a generic gen_visit_object().

The generated code for structs and for flat unions is unchanged.
For simple unions, a new visit_type_FOO_fields() is created,
wrapping the visit of the non-variant tag field:

|+static void visit_type_ChardevBackend_fields(Visitor *v, ChardevBackend **obj, Error **errp)
|+{
|+    Error *err = NULL;
|+
|+    visit_type_ChardevBackendKind(v, &(*obj)->type, "type", &err);
|+    if (err) {
|+        goto out;
|+    }
|+
|+out:
|+    error_propagate(errp, err);
|+}
|+
| void visit_type_ChardevBackend(Visitor *v, ChardevBackend **obj, const char *name, Error **errp)
| {
|     Error *err = NULL;
|@@ -2319,7 +2332,7 @@ void visit_type_ChardevBackend(Visitor *
|     if (!*obj) {
|         goto out_obj;
|     }
|-    visit_type_ChardevBackendKind(v, &(*obj)->type, "type", &err);
|+    visit_type_ChardevBackend_fields(v, obj, &err);
|     if (err) {

Signed-off-by: Eric Blake <eblake@redhat.com>

---
v6: new patch
---
 scripts/qapi-visit.py | 133 +++++++++++++++++++-------------------------------
 1 file changed, 51 insertions(+), 82 deletions(-)

diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 421a5b5..f22ebeb 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -109,46 +109,6 @@ out:
     return ret


-def gen_visit_struct(name, base, members):
-    ret = gen_visit_struct_fields(name, base, members)
-
-    # FIXME: if *obj is NULL on entry, and visit_start_struct() assigns to
-    # *obj, but then visit_type_FOO_fields() fails, we should clean up *obj
-    # rather than leaving it non-NULL. As currently written, the caller must
-    # call qapi_free_FOO() to avoid a memory leak of the partial FOO.
-    ret += mcgen('''
-
-void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error **errp)
-{
-    Error *err = NULL;
-
-    visit_start_struct(v, (void **)obj, "%(name)s", name, sizeof(%(c_name)s), &err);
-    if (err) {
-        goto out;
-    }
-    if (!*obj) {
-        goto out_obj;
-    }
-''',
-                 name=name, c_name=c_name(name))
-    if (base and not base.is_empty()) or members:
-        ret += mcgen('''
-    visit_type_%(c_name)s_fields(v, obj, &err);
-''',
-                     c_name=c_name(name))
-    ret += mcgen('''
-out_obj:
-    error_propagate(errp, err);
-    err = NULL;
-    visit_end_struct(v, &err);
-out:
-    error_propagate(errp, err);
-}
-''')
-
-    return ret
-
-
 def gen_visit_list(name, element_type):
     # FIXME: if *obj is NULL on entry, and the first visit_next_list()
     # assigns to *obj, while a later one fails, we should clean up *obj
@@ -244,18 +204,24 @@ out:
     return ret


-def gen_visit_union(name, base, variants):
+def gen_visit_object(name, base, members, variants):
     ret = ''

     assert base
     if not base.is_empty():
         ret += gen_visit_fields_decl(base)
+    if members:
+        ret += gen_visit_struct_fields(name, base, members)
+    if variants:
+        for var in variants.variants:
+            # Ugly special case for simple union TODO get rid of it
+            if not var.simple_union_type():
+                ret += gen_visit_implicit_struct(var.type)

-    for var in variants.variants:
-        # Ugly special case for simple union TODO get rid of it
-        if not var.simple_union_type():
-            ret += gen_visit_implicit_struct(var.type)
-
+    # FIXME: if *obj is NULL on entry, and visit_start_struct() assigns to
+    # *obj, but then visit_type_FOO_fields() fails, we should clean up *obj
+    # rather than leaving it non-NULL. As currently written, the caller must
+    # call qapi_free_FOO() to avoid a memory leak of the partial FOO.
     ret += mcgen('''

 void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error **errp)
@@ -272,61 +238,71 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error
 ''',
                  c_name=c_name(name), name=name)

-    if not base.is_empty():
+    if not base.is_empty() or members:
+        if members:
+            type_name = c_name(name)
+            cast = ''
+        else:
+            type_name = base.c_name()
+            cast = '(%s **)' % type_name
         ret += mcgen('''
-    visit_type_%(c_name)s_fields(v, (%(c_name)s **)obj, &err);
+    visit_type_%(c_name)s_fields(v, %(cast)sobj, &err);
 ''',
-                     c_name=base.c_name())
-    else:
+                     c_name=type_name, cast=cast)
+        if variants:
+            ret += gen_err_check(label='out_obj')
+
+    if variants:
         ret += mcgen('''
-    visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, "%(name)s", &err);
-''',
-                     c_type=variants.tag_member.type.c_name(),
-                     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)->u.data, &err) || err) {
         goto out_obj;
     }
     switch ((*obj)->%(c_name)s) {
 ''',
-                 c_name=c_name(variants.tag_member.name))
+                     c_name=c_name(variants.tag_member.name))

-    for var in variants.variants:
-        # TODO ugly special case for simple union
-        simple_union_type = var.simple_union_type()
-        ret += mcgen('''
+        for var in variants.variants:
+            # TODO ugly special case for simple union
+            simple_union_type = var.simple_union_type()
+            ret += mcgen('''
     case %(case)s:
 ''',
-                     case=c_enum_const(variants.tag_member.type.name,
-                                       var.name))
-        if simple_union_type:
-            ret += mcgen('''
+                         case=c_enum_const(variants.tag_member.type.name,
+                                           var.name))
+            if simple_union_type:
+                ret += mcgen('''
         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))
-        elif not var.type.is_empty():
-            ret += mcgen('''
+                             c_type=simple_union_type.c_name(),
+                             c_name=c_name(var.name))
+            elif not var.type.is_empty():
+                ret += mcgen('''
         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))
-        ret += mcgen('''
+                             c_type=var.type.c_name(),
+                             c_name=c_name(var.name))
+            ret += mcgen('''
         break;
 ''')

-    ret += mcgen('''
+        ret += mcgen('''
     default:
         abort();
     }
+''')
+
+    ret += mcgen('''
 out_obj:
+''')
+    if variants:
+        ret += mcgen('''
     error_propagate(errp, err);
     err = NULL;
     if (*obj) {
         visit_end_union(v, !!(*obj)->u.data, &err);
     }
+''')
+    ret += mcgen('''
     error_propagate(errp, err);
     err = NULL;
     visit_end_struct(v, &err);
@@ -387,14 +363,7 @@ class QAPISchemaGenVisitVisitor(QAPISchemaVisitor):

     def visit_object_type(self, name, info, base, members, variants):
         self.decl += gen_visit_decl(name)
-        if variants:
-            if members:
-                # Members other than variants.tag_member not implemented
-                assert len(members) == 1
-                assert members[0] == variants.tag_member
-            self.defn += gen_visit_union(name, base, variants)
-        else:
-            self.defn += gen_visit_struct(name, base, members)
+        self.defn += gen_visit_object(name, base, members, variants)

     def visit_alternate_type(self, name, info, variants):
         self.decl += gen_visit_decl(name)
-- 
2.4.3