diff mbox

[RFC,v2,33/47] qapi: Clean up after recent conversions to QAPISchemaVisitor

Message ID 1435782155-31412-34-git-send-email-armbru@redhat.com
State New
Headers show

Commit Message

Markus Armbruster July 1, 2015, 8:22 p.m. UTC 
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(-)

Comments

Eric Blake July 23, 2015, 4:48 p.m. UTC | #1 
On 07/01/2015 02:22 PM, Markus Armbruster wrote:
> 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.

Not sure if it is worth splitting this into pieces.  Fortunately, there
are no changes to generated output.

> 
> 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):

I can see how 'args' is plural (even if it is a single string for the
name of a type containing the args), but should it be 'ret' instead of
'rets'?

> @@ -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):

At least you're consistent on naming it 'rets',

> +    ret = ''

and the naming lets you distinguish between the parameter (the type
describing returned fields) and the local string (the generated C code
holding the return information).

> @@ -82,45 +76,46 @@ def generate_event_implement(api_name, event_name, params):

>  
> +            # Ugly: need to cast away the const
>              if memb.type.name == "str":
> -                var_type = "(char **)"
> +                cast = "(char **)"

And to think I called it out in a previous patch. So you noticed it too :)

Don't you want to use '(char **)' here, since it is a literal string
destined for generated C?

>              else:
> -                var_type = ""
> +                cast = ""

and '' here?

> +++ 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;

May be some churn here if you like my comment earlier in the series that
this 'struct' is redundant.

> +++ b/scripts/qapi-visit.py

> -def generate_visit_struct_fields(name, members, base = None):
> +def gen_visit_struct_fields(name, base, members):
>      struct_fields_seen.add(name)

> -                     type=base.c_name(), c_name=c_name('base'))
> +                     c_type=base.c_name(), c_name=c_name('base'))

Possible churn here based on my earlier comments about c_name(constant)
being constant.

Fairly big, but aside from some minor '' quoting issues, I didn't see
anything wrong.

Reviewed-by: Eric Blake <eblake@redhat.com>
Markus Armbruster July 28, 2015, 9:18 a.m. UTC | #2 
Eric Blake <eblake@redhat.com> writes:

> On 07/01/2015 02:22 PM, Markus Armbruster wrote:
>> 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.
>
> Not sure if it is worth splitting this into pieces.  Fortunately, there
> are no changes to generated output.

I'm afraid splitting would increase churn without much gain.  If you
want me to split, I can try.

>> 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):
>
> I can see how 'args' is plural (even if it is a single string for the
> name of a type containing the args), but should it be 'ret' instead of
> 'rets'?

I now realize readers may find this odd.

The QMP specification talks about command arguments and command
responses.

The QMP wire format uses 'arguments' and 'return'.

The schema language uses 'data' and 'returns'.  Near-perfect score on
terminology inconsistency, as usual.  Anyway, 'data' is plural (and a
rather unhelpful choice of syntax).  'returns' could either be the
plural of the noun "return", or the third person singular of the verb
"to return".

Permit me a philosophical digression.  The common way to do functions in
programming is to have multiple arguments and a single return value.  I
believe this is mostly common machines' calling conventions leaking into
languages.  From a more abstract point of view, there's no structural
difference between function arguments and values: both are simply an
element of an arbitrary set (domain and codomain, respectively).  In
particular, both can be tuples.

It's perfectly sane to have functions take exactly one argument and
yield exactly one value.  Some functional languages work that way.

But when both argument and value are generally tuples anyway, as they
are in QAPI/QMP, it's more natural to talk about arguments and return
values.  I abbreviated to args and rets.  There's method to my madness
;)

I'm open to better ideas on terminology.

>> @@ -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):
>
> At least you're consistent on naming it 'rets',
>
>> +    ret = ''
>
> and the naming lets you distinguish between the parameter (the type
> describing returned fields) and the local string (the generated C code
> holding the return information).
>
>> @@ -82,45 +76,46 @@ def generate_event_implement(api_name, event_name, params):
>
>>  
>> +            # Ugly: need to cast away the const
>>              if memb.type.name == "str":
>> -                var_type = "(char **)"
>> +                cast = "(char **)"
>
> And to think I called it out in a previous patch. So you noticed it too :)
>
> Don't you want to use '(char **)' here, since it is a literal string
> destined for generated C?

Yes.

>>              else:
>> -                var_type = ""
>> +                cast = ""
>
> and '' here?

Yes.

>> +++ 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;
>
> May be some churn here if you like my comment earlier in the series that
> this 'struct' is redundant.

Can drop it in this patch.

I'm not into typedef'ing struct/union away unless the type is opaque,
but it's the QEMU style, so let's stick to it.

>> +++ b/scripts/qapi-visit.py
>
>> -def generate_visit_struct_fields(name, members, base = None):
>> +def gen_visit_struct_fields(name, base, members):
>>      struct_fields_seen.add(name)
>
>> -                     type=base.c_name(), c_name=c_name('base'))
>> +                     c_type=base.c_name(), c_name=c_name('base'))
>
> Possible churn here based on my earlier comments about c_name(constant)
> being constant.

I'm leaning towards leaving it as is just to keep the code similar to
other places generating visit_type_FOO() calls.

> Fairly big, but aside from some minor '' quoting issues, I didn't see
> anything wrong.
>
> Reviewed-by: Eric Blake <eblake@redhat.com>

Thanks!
Eric Blake July 28, 2015, 9:13 p.m. UTC | #3 
On 07/28/2015 03:18 AM, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
> 
>> On 07/01/2015 02:22 PM, Markus Armbruster wrote:
>>> 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.
>>
>> Not sure if it is worth splitting this into pieces.  Fortunately, there
>> are no changes to generated output.
> 
> I'm afraid splitting would increase churn without much gain.  If you
> want me to split, I can try.

I can live without the split; generated output being unchanged is
already a good sign.

>>> -def generate_command_decl(name, args, ret_type):
>>> -    arglist=""
>>> +def gen_command_decl(name, args, rets):
>>
>> I can see how 'args' is plural (even if it is a single string for the
>> name of a type containing the args), but should it be 'ret' instead of
>> 'rets'?
> 
> I now realize readers may find this odd.
> 
> The QMP specification talks about command arguments and command
> responses.
> 
> The QMP wire format uses 'arguments' and 'return'.
> 
> The schema language uses 'data' and 'returns'.  Near-perfect score on
> terminology inconsistency, as usual.  Anyway, 'data' is plural (and a
> rather unhelpful choice of syntax).  'returns' could either be the
> plural of the noun "return", or the third person singular of the verb
> "to return".
> 
> Permit me a philosophical digression.  The common way to do functions in
> programming is to have multiple arguments and a single return value.  I
> believe this is mostly common machines' calling conventions leaking into
> languages.  From a more abstract point of view, there's no structural
> difference between function arguments and values: both are simply an
> element of an arbitrary set (domain and codomain, respectively).  In
> particular, both can be tuples.
> 
> It's perfectly sane to have functions take exactly one argument and
> yield exactly one value.  Some functional languages work that way.
> 
> But when both argument and value are generally tuples anyway, as they
> are in QAPI/QMP, it's more natural to talk about arguments and return
> values.  I abbreviated to args and rets.  There's method to my madness
> ;)
> 
> I'm open to better ideas on terminology.

Not sure I'm thinking of anything better; so while I found it unusual,
the explanation helps and I certainly won't reject it as wrong.

>>> -def generate_visit_struct_fields(name, members, base = None):
>>> +def gen_visit_struct_fields(name, base, members):
>>>      struct_fields_seen.add(name)
>>
>>> -                     type=base.c_name(), c_name=c_name('base'))
>>> +                     c_type=base.c_name(), c_name=c_name('base'))
>>
>> Possible churn here based on my earlier comments about c_name(constant)
>> being constant.
> 
> I'm leaning towards leaving it as is just to keep the code similar to
> other places generating visit_type_FOO() calls.

And I already easily got rid of it in my followup RFC patches, so no
problem if you leave it as is for the sake of getting this series in.
Eric Blake July 28, 2015, 9:37 p.m. UTC | #4 
On 07/28/2015 03:13 PM, Eric Blake wrote:

> 
>>>> -def generate_command_decl(name, args, ret_type):
>>>> -    arglist=""
>>>> +def gen_command_decl(name, args, rets):
>>>
>>> I can see how 'args' is plural (even if it is a single string for the
>>> name of a type containing the args), but should it be 'ret' instead of
>>> 'rets'?
>>

>> I'm open to better ideas on terminology.
> 
> Not sure I'm thinking of anything better; so while I found it unusual,
> the explanation helps and I certainly won't reject it as wrong.

Maybe gen_command_decl(name, arg_type, ret_type) since we've already
reduced the set of dictionaries into a single type name (where the type
name is possibly implicit)?  After all, we're not calling the function
with a python list or set, but with a singlar string (where that string
is the name of a type, and it is the type that then has multiple members
to form plural arguments or plural return values).  A bit more typing,
though, so up to you.
Markus Armbruster July 29, 2015, 8:33 a.m. UTC | #5 
Eric Blake <eblake@redhat.com> writes:

> On 07/28/2015 03:13 PM, Eric Blake wrote:
>
>> 
>>>>> -def generate_command_decl(name, args, ret_type):
>>>>> -    arglist=""
>>>>> +def gen_command_decl(name, args, rets):
>>>>
>>>> I can see how 'args' is plural (even if it is a single string for the
>>>> name of a type containing the args), but should it be 'ret' instead of
>>>> 'rets'?
>>>
>
>>> I'm open to better ideas on terminology.
>> 
>> Not sure I'm thinking of anything better; so while I found it unusual,
>> the explanation helps and I certainly won't reject it as wrong.
>
> Maybe gen_command_decl(name, arg_type, ret_type) since we've already
> reduced the set of dictionaries into a single type name (where the type
> name is possibly implicit)?  After all, we're not calling the function
> with a python list or set, but with a singlar string (where that string
> is the name of a type, and it is the type that then has multiple members
> to form plural arguments or plural return values).  A bit more typing,
> though, so up to you.

Okay, I'll try and see how these names pan out.
diff mbox

Patch

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