[Qemu-devel] [PATCH RFC v2 33/47] qapi: Clean up after recent conversions to QAPISchemaVisitor

[Markus Armbruster <armbru@redhat.com>]

Drop helper functions that are now unused.

Make pylint reasonably happy.

Rename generate_FOO() functions to gen_FOO() for consistency.

Use more consistent and sensible variable names.

Consistently use c_ for mapping keys when their value is a C
identifier or type.

Simplify gen_enum() and gen_visit_union()

Consistently use single quotes for C text string literals.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/qapi-commands.py | 109 +++++++++++++++++++------------------
 scripts/qapi-event.py    | 117 +++++++++++++++++++---------------------
 scripts/qapi-types.py    |  68 ++++++++++++-----------
 scripts/qapi-visit.py    | 121 ++++++++++++++++++++---------------------
 scripts/qapi.py          | 138 +++++++++--------------------------------------
 5 files changed, 229 insertions(+), 324 deletions(-)

diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index d3bddb6..5d11032 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -15,20 +15,20 @@
 from qapi import *
 import re
 
-def generate_command_decl(name, args, ret_type):
-    arglist=""
+def gen_command_decl(name, args, rets):
+    argstr = ''
     if args:
         for memb in args.members:
-            argtype = memb.type.c_type(is_param=True)
             if memb.optional:
-                arglist += "bool has_%s, " % c_name(memb.name)
-            arglist += "%s %s, " % (argtype, c_name(memb.name))
+                argstr += 'bool has_%s, ' % c_name(memb.name)
+            argstr += '%s %s, ' % (memb.type.c_type(is_param=True),
+                                   c_name(memb.name))
     return mcgen('''
-%(ret_type)s qmp_%(name)s(%(args)sError **errp);
+%(c_type)s qmp_%(c_name)s(%(args)sError **errp);
 ''',
-                 ret_type=(ret_type and ret_type.c_type()) or 'void',
-                 name=c_name(name),
-                 args=arglist)
+                 c_type=(rets and rets.c_type()) or 'void',
+                 c_name=c_name(name),
+                 args=argstr)
 
 def gen_err_check(err):
     if not err:
@@ -40,34 +40,37 @@ if (%(err)s) {
 ''',
                  err=err)
 
-def gen_sync_call(name, args, ret_type):
-    ret = ""
-    arglist=""
-    retval=""
-    if ret_type:
-        retval = "retval = "
+def gen_call(name, args, rets):
+    ret = ''
+
+    argstr = ''
     if args:
         for memb in args.members:
             if memb.optional:
-                arglist += "has_%s, " % c_name(memb.name)
-            arglist += "%s, " % c_name(memb.name)
+                argstr += 'has_%s, ' % c_name(memb.name)
+            argstr += '%s, ' % c_name(memb.name)
+
+    lhs = ''
+    if rets:
+        lhs = 'retval = '
+
     push_indent()
     ret = mcgen('''
-%(retval)sqmp_%(name)s(%(args)s&local_err);
+%(lhs)sqmp_%(c_name)s(%(args)s&local_err);
 ''',
-                name=c_name(name), args=arglist, retval=retval)
-    if ret_type:
+                c_name=c_name(name), args=argstr, lhs=lhs)
+    if rets:
         ret += gen_err_check('local_err')
         ret += mcgen('''
 
 qmp_marshal_output_%(c_name)s(retval, ret, &local_err);
 ''',
-                            c_name=c_name(name))
+                     c_name=c_name(name))
     pop_indent()
     return ret
 
 def gen_visitor_input_containers_decl(args):
-    ret = ""
+    ret = ''
 
     push_indent()
     if args:
@@ -81,16 +84,16 @@ Visitor *v;
     return ret
 
 def gen_visitor_input_vars_decl(args):
-    ret = ""
+    ret = ''
     push_indent()
 
     if args:
         for memb in args.members:
             if memb.optional:
                 ret += mcgen('''
-bool has_%(argname)s = false;
+bool has_%(c_name)s = false;
 ''',
-                             argname=c_name(memb.name))
+                             c_name=c_name(memb.name))
             ret += mcgen('''
 %(c_type)s %(c_name)s = %(c_null)s;
 ''',
@@ -102,7 +105,7 @@ bool has_%(argname)s = false;
     return ret
 
 def gen_visitor_input_block(args, dealloc=False):
-    ret = ""
+    ret = ''
     errparg = '&local_err'
     errarg = 'local_err'
 
@@ -113,7 +116,7 @@ def gen_visitor_input_block(args, dealloc=False):
 
     if dealloc:
         errparg = 'NULL'
-        errarg = None;
+        errarg = None
         ret += mcgen('''
 qmp_input_visitor_cleanup(mi);
 md = qapi_dealloc_visitor_new();
@@ -138,10 +141,10 @@ if (has_%(c_name)s) {
                          c_name=c_name(memb.name))
             push_indent()
         ret += mcgen('''
-visit_type_%(visitor)s(v, &%(c_name)s, "%(name)s", %(errp)s);
+visit_type_%(c_type)s(v, &%(c_name)s, "%(name)s", %(errp)s);
 ''',
                      c_name=c_name(memb.name), name=memb.name,
-                     visitor=memb.type.c_name(), errp=errparg)
+                     c_type=memb.type.c_name(), errp=errparg)
         ret += gen_err_check(errarg)
         if memb.optional:
             pop_indent()
@@ -156,13 +159,13 @@ qapi_dealloc_visitor_cleanup(md);
     pop_indent()
     return ret
 
-def gen_marshal_output(name, ret_type):
-    if not ret_type:
-        return ""
+def gen_marshal_output(name, rets):
+    if not rets:
+        return ''
 
     ret = mcgen('''
 
-static void qmp_marshal_output_%(c_name)s(%(c_ret_type)s ret_in, QObject **ret_out, Error **errp)
+static void qmp_marshal_output_%(c_cmd_name)s(%(c_type)s ret_in, QObject **ret_out, Error **errp)
 {
     Error *local_err = NULL;
     QmpOutputVisitor *mo = qmp_output_visitor_new();
@@ -170,7 +173,7 @@ static void qmp_marshal_output_%(c_name)s(%(c_ret_type)s ret_in, QObject **ret_o
     Visitor *v;
 
     v = qmp_output_get_visitor(mo);
-    visit_type_%(visitor)s(v, &ret_in, "unused", &local_err);
+    visit_type_%(c_name)s(v, &ret_in, "unused", &local_err);
     if (local_err) {
         goto out;
     }
@@ -181,23 +184,23 @@ out:
     qmp_output_visitor_cleanup(mo);
     md = qapi_dealloc_visitor_new();
     v = qapi_dealloc_get_visitor(md);
-    visit_type_%(visitor)s(v, &ret_in, "unused", NULL);
+    visit_type_%(c_name)s(v, &ret_in, "unused", NULL);
     qapi_dealloc_visitor_cleanup(md);
 }
 ''',
-                c_ret_type=ret_type.c_type(), c_name=c_name(name),
-                visitor=ret_type.c_name())
+                c_type=rets.c_type(), c_cmd_name=c_name(name),
+                c_name=rets.c_name())
 
     return ret
 
-def gen_marshal_input_decl(name, middle_mode):
+def gen_marshal_input_decl(name):
     ret = 'void qmp_marshal_input_%s(QDict *args, QObject **ret, Error **errp)' % c_name(name)
     if not middle_mode:
-        ret = "static " + ret
+        ret = 'static ' + ret
     return ret
 
-def gen_marshal_input(name, args, ret_type, middle_mode):
-    hdr = gen_marshal_input_decl(name, middle_mode)
+def gen_marshal_input(name, args, rets):
+    hdr = gen_marshal_input_decl(name)
 
     ret = mcgen('''
 
@@ -207,12 +210,12 @@ def gen_marshal_input(name, args, ret_type, middle_mode):
 ''',
                 header=hdr)
 
-    if ret_type:
+    if rets:
         # FIXME fishy: only pointers are initialized
-        if ret_type.c_null() == 'NULL':
-            retval = "    %s retval = NULL;" % ret_type.c_type()
+        if rets.c_null() == 'NULL':
+            retval = '    %s retval = NULL;' % rets.c_type()
         else:
-            retval = "    %s retval;" % ret_type.c_type()
+            retval = '    %s retval;' % rets.c_type()
         ret += mcgen('''
 %(retval)s
 ''',
@@ -229,9 +232,9 @@ def gen_marshal_input(name, args, ret_type, middle_mode):
 
 ''')
 
-    ret += gen_sync_call(name, args, ret_type)
+    ret += gen_call(name, args, rets)
 
-    if re.search('^ *goto out\\;', ret, re.MULTILINE):
+    if re.search('^ *goto out;', ret, re.MULTILINE):
         ret += mcgen('''
 
 out:
@@ -254,8 +257,8 @@ def gen_register_command(name, success_response):
     ret = mcgen('''
 qmp_register_command("%(name)s", qmp_marshal_input_%(c_name)s, %(opts)s);
 ''',
-                     name=name, c_name=c_name(name),
-                     opts=options)
+                name=name, c_name=c_name(name),
+                opts=options)
     pop_indent()
     return ret
 
@@ -289,12 +292,12 @@ class QAPISchemaGenCommandVisitor(QAPISchemaVisitor):
     def visit_command(self, name, info, args, rets, gen, success_response):
         if not gen:
             return
-        self.decl += generate_command_decl(name, args, rets)
+        self.decl += gen_command_decl(name, args, rets)
         if rets:
             self.defn += gen_marshal_output(name, rets)
         if middle_mode:
-            self.decl += gen_marshal_input_decl(name, middle_mode) + ';\n'
-        self.defn += gen_marshal_input(name, args, rets, middle_mode)
+            self.decl += gen_marshal_input_decl(name) + ';\n'
+        self.defn += gen_marshal_input(name, args, rets)
         if not middle_mode:
             self.regy += gen_register_command(name, success_response)
 
@@ -354,7 +357,7 @@ fdef.write(mcgen('''
 #include "%(prefix)sqmp-commands.h"
 
 ''',
-                prefix=prefix))
+                 prefix=prefix))
 
 fdecl.write(mcgen('''
 #include "%(prefix)sqapi-types.h"
diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
index 456e590..03bb1ec 100644
--- a/scripts/qapi-event.py
+++ b/scripts/qapi-event.py
@@ -13,8 +13,8 @@
 
 from qapi import *
 
-def _generate_event_api_name(event_name, data):
-    api_name = "void qapi_event_send_%s(" % c_name(event_name).lower();
+def gen_event_send_proto(name, data):
+    api_name = "void qapi_event_send_%s(" % c_name(name).lower()
     l = len(api_name)
 
     if data:
@@ -28,53 +28,47 @@ def _generate_event_api_name(event_name, data):
             api_name += "".ljust(l)
 
     api_name += "Error **errp)"
-    return api_name;
+    return api_name
 
-
-# Following are the core functions that generate C APIs to emit event.
-
-def generate_event_declaration(api_name):
+def gen_event_send_decl(name, data):
     return mcgen('''
 
-%(api_name)s;
+%(proto)s;
 ''',
-                 api_name = api_name)
+                 proto=gen_event_send_proto(name, data))
 
-def generate_event_implement(api_name, event_name, params):
-    # step 1: declare any variables
-    ret = mcgen("""
+def gen_event_send(name, data):
+    ret = mcgen('''
 
-%(api_name)s
+%(proto)s
 {
     QDict *qmp;
     Error *local_err = NULL;
     QMPEventFuncEmit emit;
-""",
-                api_name = api_name)
+''',
+                proto=gen_event_send_proto(name, data))
 
-    if params and params.members:
-        ret += mcgen("""
+    if data and data.members:
+        ret += mcgen('''
     QmpOutputVisitor *qov;
     Visitor *v;
     QObject *obj;
 
-""")
+''')
 
-    # step 2: check emit function, create a dict
-    ret += mcgen("""
+    ret += mcgen('''
     emit = qmp_event_get_func_emit();
     if (!emit) {
         return;
     }
 
-    qmp = qmp_event_build_dict("%(event_name)s");
+    qmp = qmp_event_build_dict("%(name)s");
 
-""",
-                 event_name = event_name)
+''',
+                 name=name)
 
-    # step 3: visit the params if params != None
-    if params and params.members:
-        ret += mcgen("""
+    if data and data.members:
+        ret += mcgen('''
     qov = qmp_output_visitor_new();
     g_assert(qov);
 
@@ -82,45 +76,46 @@ def generate_event_implement(api_name, event_name, params):
     g_assert(v);
 
     /* Fake visit, as if all members are under a structure */
-    visit_start_struct(v, NULL, "", "%(event_name)s", 0, &local_err);
+    visit_start_struct(v, NULL, "", "%(name)s", 0, &local_err);
     if (local_err) {
         goto clean;
     }
 
-""",
-                event_name = event_name)
+''',
+                     name=name)
 
-        for memb in params.members:
+        for memb in data.members:
             if memb.optional:
-                ret += mcgen("""
-    if (has_%(var)s) {
-""",
-                             var=c_name(memb.name))
+                ret += mcgen('''
+    if (has_%(c_name)s) {
+''',
+                             c_name=c_name(memb.name))
                 push_indent()
 
+            # Ugly: need to cast away the const
             if memb.type.name == "str":
-                var_type = "(char **)"
+                cast = "(char **)"
             else:
-                var_type = ""
+                cast = ""
 
-            ret += mcgen("""
-    visit_type_%(type)s(v, %(var_type)s&%(var)s, "%(name)s", &local_err);
+            ret += mcgen('''
+    visit_type_%(c_type)s(v, %(cast)s&%(c_name)s, "%(name)s", &local_err);
     if (local_err) {
         goto clean;
     }
-""",
-                         var_type = var_type,
-                         var=c_name(memb.name),
-                         type=memb.type.c_name(),
+''',
+                         cast=cast,
+                         c_name=c_name(memb.name),
+                         c_type=memb.type.c_name(),
                          name=memb.name)
 
             if memb.optional:
                 pop_indent()
-                ret += mcgen("""
+                ret += mcgen('''
     }
-""")
+''')
 
-        ret += mcgen("""
+        ret += mcgen('''
 
     visit_end_struct(v, &local_err);
     if (local_err) {
@@ -131,27 +126,24 @@ def generate_event_implement(api_name, event_name, params):
     g_assert(obj != NULL);
 
     qdict_put_obj(qmp, "data", obj);
-""")
+''')
 
-    # step 4: call qmp event api
-    ret += mcgen("""
-    emit(%(event_enum_value)s, qmp, &local_err);
+    ret += mcgen('''
+    emit(%(c_enum)s, qmp, &local_err);
 
-""",
-                 event_enum_value = c_enum_const(event_enum_name, event_name))
+''',
+                 c_enum=c_enum_const(event_enum_name, name))
 
-    # step 5: clean up
-    if params and params.members:
-        ret += mcgen("""
+    if data and data.members:
+        ret += mcgen('''
  clean:
     qmp_output_visitor_cleanup(qov);
-""")
-    ret += mcgen("""
+''')
+    ret += mcgen('''
     error_propagate(errp, local_err);
     QDECREF(qmp);
 }
-""")
-
+''')
     return ret
 
 class QAPISchemaGenEventVisitor(QAPISchemaVisitor):
@@ -164,12 +156,11 @@ class QAPISchemaGenEventVisitor(QAPISchemaVisitor):
         self.defn = ''
         self.event_names = []
     def visit_end(self):
-        self.decl += generate_enum(event_enum_name, self.event_names)
-        self.defn += generate_enum_lookup(event_enum_name, self.event_names)
+        self.decl += gen_enum(event_enum_name, self.event_names)
+        self.defn += gen_enum_lookup(event_enum_name, self.event_names)
     def visit_event(self, name, info, data):
-        api_name = _generate_event_api_name(name, data)
-        self.decl += generate_event_declaration(api_name)
-        self.defn += generate_event_implement(api_name, name, data)
+        self.decl += gen_event_send_decl(name, data)
+        self.defn += gen_event_send(name, data)
         self.event_names.append(name)
 
 (input_file, output_dir, do_c, do_h, prefix, dummy) = parse_command_line()
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 540c359..b6990a5 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -16,22 +16,22 @@ from qapi import *
 def gen_fwd_object_or_array(name):
     return mcgen('''
 
-typedef struct %(name)s %(name)s;
+typedef struct %(c_name)s %(c_name)s;
 ''',
-                 name=c_name(name))
+                 c_name=c_name(name))
 
 def gen_array(name, element_type):
     return mcgen('''
 
-struct %(name)s {
+struct %(c_name)s {
     union {
         %(c_type)s value;
         uint64_t padding;
     };
-    struct %(name)s *next;
+    struct %(c_name)s *next;
 };
 ''',
-                 name=c_name(name), c_type=element_type.c_type())
+                 c_name=c_name(name), c_type=element_type.c_type())
 
 def gen_struct_field(name, typ, optional):
     ret = ''
@@ -47,7 +47,7 @@ def gen_struct_field(name, typ, optional):
                  c_type=typ.c_type(), c_name=c_name(name))
     return ret
 
-def generate_struct_fields(members):
+def gen_struct_fields(members):
     ret = ''
 
     for memb in members:
@@ -57,20 +57,20 @@ def generate_struct_fields(members):
 def gen_struct(name, base, members):
     ret = mcgen('''
 
-struct %(name)s {
+struct %(c_name)s {
 ''',
-                name=c_name(name))
+                c_name=c_name(name))
 
     if base:
         ret += gen_struct_field('base', base, False)
 
-    ret += generate_struct_fields(members)
+    ret += gen_struct_fields(members)
 
     # Make sure that all structs have at least one field; this avoids
     # potential issues with attempting to malloc space for zero-length structs
     # in C, and also incompatibility with C++ (where an empty struct is size 1).
     if not base and not members:
-            ret += mcgen('''
+        ret += mcgen('''
     char qapi_dummy_field_for_empty_struct;
 ''')
 
@@ -90,9 +90,9 @@ extern const int %(c_name)s_qtypes[];
 def gen_alternate_qtypes(name, variants):
     ret = mcgen('''
 
-const int %(name)s_qtypes[QTYPE_MAX] = {
+const int %(c_name)s_qtypes[QTYPE_MAX] = {
 ''',
-                name=c_name(name))
+                c_name=c_name(name))
 
     for var in variants.variants:
         qtype = var.type.alternate_qtype()
@@ -101,7 +101,7 @@ const int %(name)s_qtypes[QTYPE_MAX] = {
         ret += mcgen('''
     [%(qtype)s] = %(enum_const)s,
 ''',
-                     qtype = qtype,
+                     qtype=qtype,
                      enum_const=c_enum_const(variants.tag_member.type.name,
                                              var.name))
 
@@ -111,25 +111,23 @@ const int %(name)s_qtypes[QTYPE_MAX] = {
     return ret
 
 def gen_union(name, base, variants):
-    name = c_name(name)
-
     ret = mcgen('''
 
-struct %(name)s {
+struct %(c_name)s {
 ''',
-                name=name)
+                c_name=c_name(name))
     if base:
-        ret += generate_struct_fields(base.members)
+        ret += gen_struct_fields(base.members)
         ret += mcgen('''
-    /* union tag is %(discriminator_type_name)s %(c_name)s */
+    /* union tag is %(c_type)s %(c_name)s */
 ''',
-                     discriminator_type_name=c_name(variants.tag_member.type.name),
+                     c_type=c_name(variants.tag_member.type.name),
                      c_name=c_name(variants.tag_member.name))
     else:
         ret += mcgen('''
-    %(discriminator_type_name)s kind;
+    %(c_type)s kind;
 ''',
-                     discriminator_type_name=c_name(variants.tag_member.type.name))
+                     c_type=c_name(variants.tag_member.type.name))
 
     ret += mcgen('''
     union {
@@ -150,18 +148,18 @@ struct %(name)s {
 
     return ret
 
-def generate_type_cleanup_decl(name):
+def gen_type_cleanup_decl(name):
     ret = mcgen('''
 
-void qapi_free_%(name)s(%(name)s *obj);
+void qapi_free_%(c_name)s(%(c_name)s *obj);
 ''',
-                name=c_name(name))
+                c_name=c_name(name))
     return ret
 
-def generate_type_cleanup(name):
+def gen_type_cleanup(name):
     ret = mcgen('''
 
-void qapi_free_%(name)s(%(name)s *obj)
+void qapi_free_%(c_name)s(%(c_name)s *obj)
 {
     QapiDeallocVisitor *md;
     Visitor *v;
@@ -172,11 +170,11 @@ void qapi_free_%(name)s(%(name)s *obj)
 
     md = qapi_dealloc_visitor_new();
     v = qapi_dealloc_get_visitor(md);
-    visit_type_%(name)s(v, &obj, NULL, NULL);
+    visit_type_%(c_name)s(v, &obj, NULL, NULL);
     qapi_dealloc_visitor_cleanup(md);
 }
 ''',
-                name=c_name(name))
+                c_name=c_name(name))
     return ret
 
 class QAPISchemaGenTypeVisitor(QAPISchemaVisitor):
@@ -207,18 +205,18 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor):
         # that have the functions defined, so generate them only with
         # option -b (do_builtins).
     def _gen_type_cleanup(self, name):
-        self.decl += generate_type_cleanup_decl(name)
-        self.defn += generate_type_cleanup(name)
+        self.decl += gen_type_cleanup_decl(name)
+        self.defn += gen_type_cleanup(name)
     def visit_enum_type(self, name, info, values):
-        self.fwdecl += generate_enum(name, values)
-        self.fwdefn += generate_enum_lookup(name, values)
+        self.fwdecl += gen_enum(name, values)
+        self.fwdefn += gen_enum_lookup(name, values)
     def visit_array_type(self, name, info, element_type):
         if isinstance(element_type, QAPISchemaBuiltinType):
             self.btin += gen_fwd_object_or_array(name)
             self.btin += gen_array(name, element_type)
-            self.btin += generate_type_cleanup_decl(name)
+            self.btin += gen_type_cleanup_decl(name)
             if do_builtins:
-                self.defn += generate_type_cleanup(name)
+                self.defn += gen_type_cleanup(name)
         else:
             self.fwdecl += gen_fwd_object_or_array(name)
             self.decl += gen_array(name, element_type)
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 135e7c1..97b1f05 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -18,18 +18,19 @@ import re
 implicit_structs_seen = set()
 struct_fields_seen = set()
 
-def generate_visit_implicit_struct(type):
-    if type in implicit_structs_seen:
+def gen_visit_implicit_struct(typ):
+    if typ in implicit_structs_seen:
         return ''
-    implicit_structs_seen.add(type)
+    implicit_structs_seen.add(typ)
+
     ret = ''
-    if type.name not in struct_fields_seen:
+    if typ.name not in struct_fields_seen:
         # Need a forward declaration
         ret += mcgen('''
 
 static void visit_type_%(c_type)s_fields(Visitor *m, %(c_type)s **obj, Error **errp);
 ''',
-                     c_type=type.c_name())
+                     c_type=typ.c_name())
 
     ret += mcgen('''
 
@@ -45,35 +46,35 @@ static void visit_type_implicit_%(c_type)s(Visitor *m, %(c_type)s **obj, Error *
     error_propagate(errp, err);
 }
 ''',
-                 c_type=type.c_name())
+                 c_type=typ.c_name())
     return ret
 
-def generate_visit_struct_fields(name, members, base = None):
+def gen_visit_struct_fields(name, base, members):
     struct_fields_seen.add(name)
 
     ret = ''
 
     if base:
-        ret += generate_visit_implicit_struct(base)
+        ret += gen_visit_implicit_struct(base)
 
     ret += mcgen('''
 
-static void visit_type_%(name)s_fields(Visitor *m, %(name)s **obj, Error **errp)
+static void visit_type_%(c_name)s_fields(Visitor *m, %(c_name)s **obj, Error **errp)
 {
     Error *err = NULL;
 
 ''',
-                 name=c_name(name))
+                 c_name=c_name(name))
     push_indent()
 
     if base:
         ret += mcgen('''
-visit_type_implicit_%(type)s(m, &(*obj)->%(c_name)s, &err);
+visit_type_implicit_%(c_type)s(m, &(*obj)->%(c_name)s, &err);
 if (err) {
     goto out;
 }
 ''',
-                     type=base.c_name(), c_name=c_name('base'))
+                     c_type=base.c_name(), c_name=c_name('base'))
 
     for memb in members:
         if memb.optional:
@@ -85,9 +86,9 @@ if (!err && (*obj)->has_%(c_name)s) {
             push_indent()
 
         ret += mcgen('''
-visit_type_%(type)s(m, &(*obj)->%(c_name)s, "%(name)s", &err);
+visit_type_%(c_type)s(m, &(*obj)->%(c_name)s, "%(name)s", &err);
 ''',
-                     type=memb.type.c_name(), c_name=c_name(memb.name),
+                     c_type=memb.type.c_name(), c_name=c_name(memb.name),
                      name=memb.name)
 
         if memb.optional:
@@ -102,7 +103,7 @@ if (err) {
 ''')
 
     pop_indent()
-    if re.search('^ *goto out\\;', ret, re.MULTILINE):
+    if re.search('^ *goto out;', ret, re.MULTILINE):
         ret += mcgen('''
 
 out:
@@ -113,8 +114,7 @@ out:
 ''')
     return ret
 
-
-def generate_visit_struct_body(name):
+def gen_visit_struct_body(name):
     ret = mcgen('''
     Error *err = NULL;
 
@@ -128,20 +128,18 @@ def generate_visit_struct_body(name):
     error_propagate(errp, err);
 ''',
                 name=name, c_name=c_name(name))
-
     return ret
 
 def gen_visit_struct(name, base, members):
-    ret = generate_visit_struct_fields(name, members, base)
-
+    ret = gen_visit_struct_fields(name, base, members)
     ret += mcgen('''
 
-void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error **errp)
+void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, Error **errp)
 {
 ''',
-                 name=c_name(name))
+                 c_name=c_name(name))
 
-    ret += generate_visit_struct_body(name)
+    ret += gen_visit_struct_body(name)
 
     ret += mcgen('''
 }
@@ -151,7 +149,7 @@ void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error **e
 def gen_visit_list(name, element_type):
     return mcgen('''
 
-void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error **errp)
+void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, Error **errp)
 {
     Error *err = NULL;
     GenericList *i, **prev;
@@ -164,7 +162,7 @@ void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error **e
     for (prev = (GenericList **)obj;
          !err && (i = visit_next_list(m, prev, &err)) != NULL;
          prev = &i) {
-        %(name)s *native_i = (%(name)s *)i;
+        %(c_name)s *native_i = (%(c_name)s *)i;
         visit_type_%(c_elt_type)s(m, &native_i->value, NULL, &err);
     }
 
@@ -175,10 +173,9 @@ out:
     error_propagate(errp, err);
 }
 ''',
-                 name=c_name(name),
-                 c_elt_type=element_type.c_name())
+                 c_name=c_name(name), c_elt_type=element_type.c_name())
 
-def generate_visit_enum(name):
+def gen_visit_enum(name):
     return mcgen('''
 
 void visit_type_%(c_name)s(Visitor *m, %(c_name)s *obj, const char *name, Error **errp)
@@ -191,33 +188,32 @@ void visit_type_%(c_name)s(Visitor *m, %(c_name)s *obj, const char *name, Error
 def gen_visit_alternate(name, variants):
     ret = mcgen('''
 
-void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error **errp)
+void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, Error **errp)
 {
     Error *err = NULL;
 
-    visit_start_implicit_struct(m, (void**) obj, sizeof(%(name)s), &err);
+    visit_start_implicit_struct(m, (void**) obj, sizeof(%(c_name)s), &err);
     if (err) {
         goto out;
     }
-    visit_get_next_type(m, (int*) &(*obj)->kind, %(name)s_qtypes, name, &err);
+    visit_get_next_type(m, (int*) &(*obj)->kind, %(c_name)s_qtypes, name, &err);
     if (err) {
         goto out_end;
     }
     switch ((*obj)->kind) {
 ''',
-                name=c_name(name))
+                c_name=c_name(name))
 
     for var in variants.variants:
-        enum_full_value = c_enum_const(variants.tag_member.type.name,
-                                       var.name)
         ret += mcgen('''
-    case %(enum_full_value)s:
+    case %(case)s:
         visit_type_%(c_type)s(m, &(*obj)->%(c_name)s, name, &err);
         break;
 ''',
-                enum_full_value = enum_full_value,
-                c_type=var.type.c_name(),
-                c_name=c_name(var.name))
+                     case=c_enum_const(variants.tag_member.type.name,
+                                       var.name),
+                     c_type=var.type.c_name(),
+                     c_name=c_name(var.name))
 
     ret += mcgen('''
     default:
@@ -239,11 +235,11 @@ def gen_visit_union(name, base, variants):
 
     if base:
         members = [m for m in base.members if m != variants.tag_member]
-        ret += generate_visit_struct_fields(name, members)
+        ret += gen_visit_struct_fields(name, None, members)
 
     for var in variants.variants:
         if var.flat:
-            ret += generate_visit_implicit_struct(var.type)
+            ret += gen_visit_implicit_struct(var.type)
 
     ret += mcgen('''
 
@@ -261,19 +257,19 @@ void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, Error
 
     if base:
         ret += mcgen('''
-        visit_type_%(name)s_fields(m, obj, &err);
+        visit_type_%(c_name)s_fields(m, obj, &err);
         if (err) {
             goto out_obj;
         }
 ''',
-                     name=c_name(name))
+                     c_name=c_name(name))
 
-    disc_key = variants.tag_member.name
+    tag_key = variants.tag_member.name
     if not variants.tag_name:
         # we pointlessly use a different key for simple unions
-        disc_key = 'type'
+        tag_key = 'type'
     ret += mcgen('''
-        visit_type_%(disc_type)s(m, &(*obj)->%(c_name)s, "%(disc_key)s", &err);
+        visit_type_%(c_type)s(m, &(*obj)->%(c_name)s, "%(name)s", &err);
         if (err) {
             goto out_obj;
         }
@@ -282,25 +278,30 @@ void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, Error
         }
         switch ((*obj)->%(c_name)s) {
 ''',
-                 disc_type=variants.tag_member.type.c_name(),
+                 c_type=variants.tag_member.type.c_name(),
                  c_name=c_name(variants.tag_member.name),
-                 disc_key = disc_key)
+                 name=tag_key)
 
     for var in variants.variants:
-        if not var.flat:
-            fmt = 'visit_type_%(c_type)s(m, &(*obj)->%(c_name)s, "data", &err);'
+        ret += mcgen('''
+        case %(case)s:
+''',
+                     case=c_enum_const(variants.tag_member.type.name, var.name))
+        if var.flat:
+            ret += mcgen('''
+            visit_type_implicit_%(c_type)s(m, &(*obj)->%(c_name)s, &err);
+''',
+                         c_type=var.type.c_name(),
+                         c_name=c_name(var.name))
         else:
-            fmt = 'visit_type_implicit_%(c_type)s(m, &(*obj)->%(c_name)s, &err);'
-
-        enum_full_value = c_enum_const(variants.tag_member.type.name, var.name)
+            ret += mcgen('''
+            visit_type_%(c_type)s(m, &(*obj)->%(c_name)s, "data", &err);
+''',
+                         c_type=var.type.c_name(),
+                         c_name=c_name(var.name))
         ret += mcgen('''
-        case %(enum_full_value)s:
-            ''' + fmt + '''
             break;
-''',
-                enum_full_value = enum_full_value,
-                c_type=var.type.c_name(),
-                c_name=c_name(var.name))
+''')
 
     ret += mcgen('''
         default:
@@ -351,7 +352,7 @@ class QAPISchemaGenVisitVisitor(QAPISchemaVisitor):
         # do_builtins (option -b) to provide control
     def visit_enum_type(self, name, info, values):
         self.decl += gen_visit_decl(name, scalar=True)
-        self.defn += generate_visit_enum(name)
+        self.defn += gen_visit_enum(name)
     def visit_array_type(self, name, info, element_type):
         decl = gen_visit_decl(name)
         defn = gen_visit_list(name, element_type)
@@ -419,7 +420,7 @@ fdef.write(mcgen('''
 #include "qemu-common.h"
 #include "%(prefix)sqapi-visit.h"
 ''',
-                 prefix = prefix))
+                 prefix=prefix))
 
 fdecl.write(mcgen('''
 #include "qapi/visitor.h"
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 699a656..4d47214 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -1016,16 +1016,6 @@ class QAPISchema(object):
         self._def_exprs()
         self.check()
 
-    def get_exprs(self):
-        def expr_name(expr):
-            name = expr.get('enum') or expr.get('union') \
-                   or expr.get('alternate') or expr.get('struct') \
-                   or expr.get('command') or expr.get('event')
-            assert name
-            return name
-        return sorted([expr_elem['expr'] for expr_elem in self.exprs],
-                      key=expr_name)
-
     def _def_entity(self, ent):
         assert ent.name not in self.entity_dict
         self.entity_dict[ent.name] = ent
@@ -1207,23 +1197,6 @@ class QAPISchema(object):
 # Code generation helpers
 #
 
-def parse_args(typeinfo):
-    if isinstance(typeinfo, str):
-        struct = find_struct(typeinfo)
-        assert struct != None
-        typeinfo = struct['data']
-
-    for member in typeinfo:
-        argname = member
-        argentry = typeinfo[member]
-        optional = False
-        if member.startswith('*'):
-            argname = member[1:]
-            optional = True
-        # Todo: allow argentry to be OrderedDict, for providing the
-        # value of an optional argument.
-        yield (argname, argentry, optional)
-
 def camel_case(name):
     new_name = ''
     first = True
@@ -1304,67 +1277,9 @@ def c_name(name, protect=True):
         return "q_" + name
     return name.translate(c_name_trans)
 
-# Map type @name to the C typedef name for the list form.
-#
-# ['Name'] -> 'NameList', ['x-Foo'] -> 'x_FooList', ['int'] -> 'intList'
-def c_list_type(name):
-    return type_name(name) + 'List'
-
-# Map type @value to the C typedef form.
-#
-# Used for converting 'type' from a 'member':'type' qapi definition
-# into the alphanumeric portion of the type for a generated C parameter,
-# as well as generated C function names.  See c_type() for the rest of
-# the conversion such as adding '*' on pointer types.
-# 'int' -> 'int', '[x-Foo]' -> 'x_FooList', '__a.b_c' -> '__a_b_c'
-def type_name(value):
-    if type(value) == list:
-        return c_list_type(value[0])
-    if value in builtin_types.keys():
-        return value
-    return c_name(value)
-
 eatspace = '\033EATSPACE.'
 pointer_suffix = ' *' + eatspace
 
-# Map type @name to its C type expression.
-# If @is_param, const-qualify the string type.
-#
-# This function is used for computing the full C type of 'member':'name'.
-# A special suffix is added in c_type() for pointer types, and it's
-# stripped in mcgen(). So please notice this when you check the return
-# value of c_type() outside mcgen().
-def c_type(value, is_param=False):
-    if value == 'str':
-        if is_param:
-            return 'const char' + pointer_suffix
-        return 'char' + pointer_suffix
-
-    elif value == 'int':
-        return 'int64_t'
-    elif (value == 'int8' or value == 'int16' or value == 'int32' or
-          value == 'int64' or value == 'uint8' or value == 'uint16' or
-          value == 'uint32' or value == 'uint64'):
-        return value + '_t'
-    elif value == 'size':
-        return 'uint64_t'
-    elif value == 'bool':
-        return 'bool'
-    elif value == 'number':
-        return 'double'
-    elif type(value) == list:
-        return c_list_type(value[0]) + pointer_suffix
-    elif is_enum(value):
-        return c_name(value)
-    elif value == None:
-        return 'void'
-    elif value in events:
-        return camel_case(value) + 'Event' + pointer_suffix
-    else:
-        # complex type name
-        assert isinstance(value, str) and value != ""
-        return c_name(value) + pointer_suffix
-
 def genindent(count):
     ret = ""
     for i in range(count):
@@ -1417,60 +1332,57 @@ def guardend(name):
 ''',
                  name=guardname(name))
 
-def generate_enum_lookup(name, values):
+def gen_enum_lookup(name, values):
     ret = mcgen('''
 
-const char *const %(name)s_lookup[] = {
+const char *const %(c_name)s_lookup[] = {
 ''',
-                name=c_name(name))
+                c_name=c_name(name))
     for value in values:
         index = c_enum_const(name, value)
         ret += mcgen('''
     [%(index)s] = "%(value)s",
 ''',
-                     index = index, value = value)
+                     index=index, value=value)
 
     max_index = c_enum_const(name, 'MAX')
     ret += mcgen('''
     [%(max_index)s] = NULL,
 };
 ''',
-        max_index=max_index)
+                 max_index=max_index)
     return ret
 
-def generate_enum(name, values):
-    name = c_name(name)
-    lookup_decl = mcgen('''
-
-extern const char *const %(name)s_lookup[];
-''',
-                name=name)
-
-    enum_decl = mcgen('''
-
-typedef enum %(name)s {
-''',
-                name=name)
-
+def gen_enum(name, values):
     # append automatically generated _MAX value
-    enum_values = values + [ 'MAX' ]
+    enum_values = values + ['MAX']
+
+    ret = mcgen('''
+
+typedef enum %(c_name)s {
+''',
+                c_name=c_name(name))
 
     i = 0
     for value in enum_values:
-        enum_full_value = c_enum_const(name, value)
-        enum_decl += mcgen('''
-    %(enum_full_value)s = %(i)d,
+        ret += mcgen('''
+    %(c_enum)s = %(i)d,
 ''',
-                     enum_full_value = enum_full_value,
+                     c_enum=c_enum_const(name, value),
                      i=i)
         i += 1
 
-    enum_decl += mcgen('''
-} %(name)s;
+    ret += mcgen('''
+} %(c_name)s;
 ''',
-                 name=name)
+                 c_name=c_name(name))
 
-    return enum_decl + lookup_decl
+    ret += mcgen('''
+
+extern const char *const %(c_name)s_lookup[];
+''',
+                 c_name=c_name(name))
+    return ret
 
 #
 # Common command line parsing
-- 
1.9.3