summary refs log tree commit diff stats
path: root/scripts
diff options
context:
space:
mode:
authorPeter Maydell <peter.maydell@linaro.org>2015-10-13 10:42:06 +0100
committerPeter Maydell <peter.maydell@linaro.org>2015-10-13 10:42:06 +0100
commitc49d3411faae8ffaab8f7e5db47405a008411c10 (patch)
treec1d186eb113f2da856120c7880574df34efca228 /scripts
parent5451316ed07b758a187dedf21047bed8f843f7f1 (diff)
parent18bdbc3ac8b477e160d56aa6ecd6942495ce44d0 (diff)
downloadfocaccia-qemu-c49d3411faae8ffaab8f7e5db47405a008411c10.tar.gz
focaccia-qemu-c49d3411faae8ffaab8f7e5db47405a008411c10.zip
Merge remote-tracking branch 'remotes/armbru/tags/pull-qapi-2015-10-12' into staging
QAPI patches

# gpg: Signature made Mon 12 Oct 2015 18:56:35 BST using RSA key ID EB918653
# gpg: Good signature from "Markus Armbruster <armbru@redhat.com>"
# gpg:                 aka "Markus Armbruster <armbru@pond.sub.org>"

* remotes/armbru/tags/pull-qapi-2015-10-12:
  qapi: Simplify gen_visit_fields() error handling
  qapi: Share gen_visit_fields()
  qapi: Share gen_err_check()
  qapi: Consistent generated code: minimize push_indent() usage
  qapi: Consistent generated code: prefer common indentation
  qapi: Consistent generated code: prefer common labels
  qapi: Consistent generated code: prefer visitor 'v'
  qapi: Consistent generated code: prefer error 'err'
  qapi: Reuse code for flat union base validation
  qapi: Test use of 'number' within alternates
  qapi: Add tests for empty unions
  qapi: Avoid assertion failure on union 'type' collision
  qapi: Test for various name collisions
  qapi: Clean up qapi.py per pep8
  qapi: Invoke exception superclass initializer
  qapi: Improve 'include' error message
  qapi: Sort qapi-schema tests
  MAINTAINERS: Specify QAPI include and test files
  MAINTAINERS: Specify QObject include and test files
  docs: Move files from docs/qmp/ to docs/

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Diffstat (limited to 'scripts')
-rw-r--r--scripts/ordereddict.py3
-rw-r--r--scripts/qapi-commands.py110
-rw-r--r--scripts/qapi-event.py55
-rw-r--r--scripts/qapi-types.py8
-rw-r--r--scripts/qapi-visit.py141
-rw-r--r--scripts/qapi.py240
6 files changed, 277 insertions, 280 deletions
diff --git a/scripts/ordereddict.py b/scripts/ordereddict.py
index 7242b5060d..2d1d81370b 100644
--- a/scripts/ordereddict.py
+++ b/scripts/ordereddict.py
@@ -22,6 +22,7 @@
 
 from UserDict import DictMixin
 
+
 class OrderedDict(dict, DictMixin):
 
     def __init__(self, *args, **kwds):
@@ -117,7 +118,7 @@ class OrderedDict(dict, DictMixin):
         if isinstance(other, OrderedDict):
             if len(self) != len(other):
                 return False
-            for p, q in  zip(self.items(), other.items()):
+            for p, q in zip(self.items(), other.items()):
                 if p != q:
                     return False
             return True
diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 810a897625..43a893b4eb 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -25,17 +25,6 @@ def gen_command_decl(name, arg_type, ret_type):
                  params=gen_params(arg_type, 'Error **errp'))
 
 
-def gen_err_check(err):
-    if not err:
-        return ''
-    return mcgen('''
-if (%(err)s) {
-    goto out;
-}
-''',
-                 err=err)
-
-
 def gen_call(name, arg_type, ret_type):
     ret = ''
 
@@ -50,51 +39,47 @@ def gen_call(name, arg_type, ret_type):
     if ret_type:
         lhs = 'retval = '
 
-    push_indent()
     ret = mcgen('''
 
-%(lhs)sqmp_%(c_name)s(%(args)s&local_err);
+    %(lhs)sqmp_%(c_name)s(%(args)s&err);
 ''',
                 c_name=c_name(name), args=argstr, lhs=lhs)
     if ret_type:
-        ret += gen_err_check('local_err')
+        ret += gen_err_check()
         ret += mcgen('''
 
-qmp_marshal_output_%(c_name)s(retval, ret, &local_err);
+    qmp_marshal_output_%(c_name)s(retval, ret, &err);
 ''',
                      c_name=ret_type.c_name())
-    pop_indent()
     return ret
 
 
 def gen_marshal_vars(arg_type, ret_type):
     ret = mcgen('''
-    Error *local_err = NULL;
+    Error *err = NULL;
 ''')
 
-    push_indent()
-
     if ret_type:
         ret += mcgen('''
-%(c_type)s retval;
+    %(c_type)s retval;
 ''',
                      c_type=ret_type.c_type())
 
     if arg_type:
         ret += mcgen('''
-QmpInputVisitor *mi = qmp_input_visitor_new_strict(QOBJECT(args));
-QapiDeallocVisitor *md;
-Visitor *v;
+    QmpInputVisitor *qiv = qmp_input_visitor_new_strict(QOBJECT(args));
+    QapiDeallocVisitor *qdv;
+    Visitor *v;
 ''')
 
         for memb in arg_type.members:
             if memb.optional:
                 ret += mcgen('''
-bool has_%(c_name)s = false;
+    bool has_%(c_name)s = false;
 ''',
                              c_name=c_name(memb.name))
             ret += mcgen('''
-%(c_type)s %(c_name)s = %(c_null)s;
+    %(c_type)s %(c_name)s = %(c_null)s;
 ''',
                          c_name=c_name(memb.name),
                          c_type=memb.type.c_type(),
@@ -103,10 +88,9 @@ bool has_%(c_name)s = false;
     else:
         ret += mcgen('''
 
-(void)args;
+    (void)args;
 ''')
 
-    pop_indent()
     return ret
 
 
@@ -116,53 +100,23 @@ def gen_marshal_input_visit(arg_type, dealloc=False):
     if not arg_type:
         return ret
 
-    push_indent()
-
     if dealloc:
-        errparg = 'NULL'
-        errarg = None
         ret += mcgen('''
-qmp_input_visitor_cleanup(mi);
-md = qapi_dealloc_visitor_new();
-v = qapi_dealloc_get_visitor(md);
+    qmp_input_visitor_cleanup(qiv);
+    qdv = qapi_dealloc_visitor_new();
+    v = qapi_dealloc_get_visitor(qdv);
 ''')
     else:
-        errparg = '&local_err'
-        errarg = 'local_err'
         ret += mcgen('''
-v = qmp_input_get_visitor(mi);
+    v = qmp_input_get_visitor(qiv);
 ''')
 
-    for memb in arg_type.members:
-        if memb.optional:
-            ret += mcgen('''
-visit_optional(v, &has_%(c_name)s, "%(name)s", %(errp)s);
-''',
-                         c_name=c_name(memb.name), name=memb.name,
-                         errp=errparg)
-            ret += gen_err_check(errarg)
-            ret += mcgen('''
-if (has_%(c_name)s) {
-''',
-                         c_name=c_name(memb.name))
-            push_indent()
-        ret += mcgen('''
-visit_type_%(c_type)s(v, &%(c_name)s, "%(name)s", %(errp)s);
-''',
-                     c_name=c_name(memb.name), name=memb.name,
-                     c_type=memb.type.c_name(), errp=errparg)
-        ret += gen_err_check(errarg)
-        if memb.optional:
-            pop_indent()
-            ret += mcgen('''
-}
-''')
+    ret += gen_visit_fields(arg_type.members, skiperr=dealloc)
 
     if dealloc:
         ret += mcgen('''
-qapi_dealloc_visitor_cleanup(md);
+    qapi_dealloc_visitor_cleanup(qdv);
 ''')
-    pop_indent()
     return ret
 
 
@@ -171,25 +125,25 @@ def gen_marshal_output(ret_type):
 
 static void qmp_marshal_output_%(c_name)s(%(c_type)s ret_in, QObject **ret_out, Error **errp)
 {
-    Error *local_err = NULL;
-    QmpOutputVisitor *mo = qmp_output_visitor_new();
-    QapiDeallocVisitor *md;
+    Error *err = NULL;
+    QmpOutputVisitor *qov = qmp_output_visitor_new();
+    QapiDeallocVisitor *qdv;
     Visitor *v;
 
-    v = qmp_output_get_visitor(mo);
-    visit_type_%(c_name)s(v, &ret_in, "unused", &local_err);
-    if (local_err) {
+    v = qmp_output_get_visitor(qov);
+    visit_type_%(c_name)s(v, &ret_in, "unused", &err);
+    if (err) {
         goto out;
     }
-    *ret_out = qmp_output_get_qobject(mo);
+    *ret_out = qmp_output_get_qobject(qov);
 
 out:
-    error_propagate(errp, local_err);
-    qmp_output_visitor_cleanup(mo);
-    md = qapi_dealloc_visitor_new();
-    v = qapi_dealloc_get_visitor(md);
+    error_propagate(errp, err);
+    qmp_output_visitor_cleanup(qov);
+    qdv = qapi_dealloc_visitor_new();
+    v = qapi_dealloc_get_visitor(qdv);
     visit_type_%(c_name)s(v, &ret_in, "unused", NULL);
-    qapi_dealloc_visitor_cleanup(md);
+    qapi_dealloc_visitor_cleanup(qdv);
 }
 ''',
                  c_type=ret_type.c_type(), c_name=ret_type.c_name())
@@ -227,7 +181,7 @@ def gen_marshal(name, arg_type, ret_type):
 out:
 ''')
     ret += mcgen('''
-    error_propagate(errp, local_err);
+    error_propagate(errp, err);
 ''')
     ret += gen_marshal_input_visit(arg_type, dealloc=True)
     ret += mcgen('''
@@ -237,17 +191,15 @@ out:
 
 
 def gen_register_command(name, success_response):
-    push_indent()
     options = 'QCO_NO_OPTIONS'
     if not success_response:
         options = 'QCO_NO_SUCCESS_RESP'
 
     ret = mcgen('''
-qmp_register_command("%(name)s", qmp_marshal_%(c_name)s, %(opts)s);
+    qmp_register_command("%(name)s", qmp_marshal_%(c_name)s, %(opts)s);
 ''',
                 name=name, c_name=c_name(name),
                 opts=options)
-    pop_indent()
     return ret
 
 
diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
index d15fad98f3..720486f06c 100644
--- a/scripts/qapi-event.py
+++ b/scripts/qapi-event.py
@@ -34,7 +34,7 @@ def gen_event_send(name, arg_type):
 %(proto)s
 {
     QDict *qmp;
-    Error *local_err = NULL;
+    Error *err = NULL;
     QMPEventFuncEmit emit;
 ''',
                 proto=gen_event_send_proto(name, arg_type))
@@ -67,50 +67,15 @@ def gen_event_send(name, arg_type):
     g_assert(v);
 
     /* Fake visit, as if all members are under a structure */
-    visit_start_struct(v, NULL, "", "%(name)s", 0, &local_err);
-    if (local_err) {
-        goto clean;
-    }
-
+    visit_start_struct(v, NULL, "", "%(name)s", 0, &err);
 ''',
                      name=name)
-
-        for memb in arg_type.members:
-            if memb.optional:
-                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":
-                cast = '(char **)'
-            else:
-                cast = ''
-
-            ret += mcgen('''
-    visit_type_%(c_type)s(v, %(cast)s&%(c_name)s, "%(name)s", &local_err);
-    if (local_err) {
-        goto clean;
-    }
-''',
-                         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 += gen_err_check()
+        ret += gen_visit_fields(arg_type.members, need_cast=True)
         ret += mcgen('''
-
-    visit_end_struct(v, &local_err);
-    if (local_err) {
-        goto clean;
+    visit_end_struct(v, &err);
+    if (err) {
+        goto out;
     }
 
     obj = qmp_output_get_qobject(qov);
@@ -120,18 +85,18 @@ def gen_event_send(name, arg_type):
 ''')
 
     ret += mcgen('''
-    emit(%(c_enum)s, qmp, &local_err);
+    emit(%(c_enum)s, qmp, &err);
 
 ''',
                  c_enum=c_enum_const(event_enum_name, name))
 
     if arg_type and arg_type.members:
         ret += mcgen('''
- clean:
+out:
     qmp_output_visitor_cleanup(qov);
 ''')
     ret += mcgen('''
-    error_propagate(errp, local_err);
+    error_propagate(errp, err);
     QDECREF(qmp);
 }
 ''')
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index b292682df6..d405f8d670 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -188,17 +188,17 @@ def gen_type_cleanup(name):
 
 void qapi_free_%(c_name)s(%(c_name)s *obj)
 {
-    QapiDeallocVisitor *md;
+    QapiDeallocVisitor *qdv;
     Visitor *v;
 
     if (!obj) {
         return;
     }
 
-    md = qapi_dealloc_visitor_new();
-    v = qapi_dealloc_get_visitor(md);
+    qdv = qapi_dealloc_visitor_new();
+    v = qapi_dealloc_get_visitor(qdv);
     visit_type_%(c_name)s(v, &obj, NULL, NULL);
-    qapi_dealloc_visitor_cleanup(md);
+    qapi_dealloc_visitor_cleanup(qdv);
 }
 ''',
                 c_name=c_name(name))
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 97343cf7e9..4f97781348 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -24,7 +24,7 @@ def gen_visit_decl(name, scalar=False):
     if not scalar:
         c_type += '*'
     return mcgen('''
-void visit_type_%(c_name)s(Visitor *m, %(c_type)sobj, const char *name, Error **errp);
+void visit_type_%(c_name)s(Visitor *v, %(c_type)sobj, const char *name, Error **errp);
 ''',
                  c_name=c_name(name), c_type=c_type)
 
@@ -39,20 +39,20 @@ def gen_visit_implicit_struct(typ):
         # Need a forward declaration
         ret += mcgen('''
 
-static void visit_type_%(c_type)s_fields(Visitor *m, %(c_type)s **obj, Error **errp);
+static void visit_type_%(c_type)s_fields(Visitor *v, %(c_type)s **obj, Error **errp);
 ''',
                      c_type=typ.c_name())
 
     ret += mcgen('''
 
-static void visit_type_implicit_%(c_type)s(Visitor *m, %(c_type)s **obj, Error **errp)
+static void visit_type_implicit_%(c_type)s(Visitor *v, %(c_type)s **obj, Error **errp)
 {
     Error *err = NULL;
 
-    visit_start_implicit_struct(m, (void **)obj, sizeof(%(c_type)s), &err);
+    visit_start_implicit_struct(v, (void **)obj, sizeof(%(c_type)s), &err);
     if (!err) {
-        visit_type_%(c_type)s_fields(m, obj, errp);
-        visit_end_implicit_struct(m, &err);
+        visit_type_%(c_type)s_fields(v, obj, errp);
+        visit_end_implicit_struct(v, &err);
     }
     error_propagate(errp, err);
 }
@@ -71,50 +71,22 @@ def gen_visit_struct_fields(name, base, members):
 
     ret += mcgen('''
 
-static void visit_type_%(c_name)s_fields(Visitor *m, %(c_name)s **obj, Error **errp)
+static void visit_type_%(c_name)s_fields(Visitor *v, %(c_name)s **obj, Error **errp)
 {
     Error *err = NULL;
 
 ''',
                  c_name=c_name(name))
-    push_indent()
 
     if base:
         ret += mcgen('''
-visit_type_implicit_%(c_type)s(m, &(*obj)->%(c_name)s, &err);
-if (err) {
-    goto out;
-}
+    visit_type_implicit_%(c_type)s(v, &(*obj)->%(c_name)s, &err);
 ''',
                      c_type=base.c_name(), c_name=c_name('base'))
+        ret += gen_err_check()
 
-    for memb in members:
-        if memb.optional:
-            ret += mcgen('''
-visit_optional(m, &(*obj)->has_%(c_name)s, "%(name)s", &err);
-if (!err && (*obj)->has_%(c_name)s) {
-''',
-                         c_name=c_name(memb.name), name=memb.name)
-            push_indent()
-
-        ret += mcgen('''
-visit_type_%(c_type)s(m, &(*obj)->%(c_name)s, "%(name)s", &err);
-''',
-                     c_type=memb.type.c_name(), c_name=c_name(memb.name),
-                     name=memb.name)
-
-        if memb.optional:
-            pop_indent()
-            ret += mcgen('''
-}
-''')
-        ret += mcgen('''
-if (err) {
-    goto out;
-}
-''')
+    ret += gen_visit_fields(members, prefix='(*obj)->')
 
-    pop_indent()
     if re.search('^ *goto out;', ret, re.MULTILINE):
         ret += mcgen('''
 
@@ -136,16 +108,16 @@ def gen_visit_struct(name, base, members):
     # call qapi_free_FOO() to avoid a memory leak of the partial FOO.
     ret += mcgen('''
 
-void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, Error **errp)
+void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error **errp)
 {
     Error *err = NULL;
 
-    visit_start_struct(m, (void **)obj, "%(name)s", name, sizeof(%(c_name)s), &err);
+    visit_start_struct(v, (void **)obj, "%(name)s", name, sizeof(%(c_name)s), &err);
     if (!err) {
         if (*obj) {
-            visit_type_%(c_name)s_fields(m, obj, errp);
+            visit_type_%(c_name)s_fields(v, obj, errp);
         }
-        visit_end_struct(m, &err);
+        visit_end_struct(v, &err);
     }
     error_propagate(errp, err);
 }
@@ -158,26 +130,26 @@ void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, Error
 def gen_visit_list(name, element_type):
     return mcgen('''
 
-void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, Error **errp)
+void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error **errp)
 {
     Error *err = NULL;
     GenericList *i, **prev;
 
-    visit_start_list(m, name, &err);
+    visit_start_list(v, name, &err);
     if (err) {
         goto out;
     }
 
     for (prev = (GenericList **)obj;
-         !err && (i = visit_next_list(m, prev, &err)) != NULL;
+         !err && (i = visit_next_list(v, prev, &err)) != NULL;
          prev = &i) {
         %(c_name)s *native_i = (%(c_name)s *)i;
-        visit_type_%(c_elt_type)s(m, &native_i->value, NULL, &err);
+        visit_type_%(c_elt_type)s(v, &native_i->value, NULL, &err);
     }
 
     error_propagate(errp, err);
     err = NULL;
-    visit_end_list(m, &err);
+    visit_end_list(v, &err);
 out:
     error_propagate(errp, err);
 }
@@ -188,9 +160,9 @@ out:
 def gen_visit_enum(name):
     return mcgen('''
 
-void visit_type_%(c_name)s(Visitor *m, %(c_name)s *obj, const char *name, Error **errp)
+void visit_type_%(c_name)s(Visitor *v, %(c_name)s *obj, const char *name, Error **errp)
 {
-    visit_type_enum(m, (int *)obj, %(c_name)s_lookup, "%(name)s", name, errp);
+    visit_type_enum(v, (int *)obj, %(c_name)s_lookup, "%(name)s", name, errp);
 }
 ''',
                  c_name=c_name(name), name=name)
@@ -199,17 +171,17 @@ 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_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, Error **errp)
+void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error **errp)
 {
     Error *err = NULL;
 
-    visit_start_implicit_struct(m, (void**) obj, sizeof(%(c_name)s), &err);
+    visit_start_implicit_struct(v, (void**) obj, sizeof(%(c_name)s), &err);
     if (err) {
         goto out;
     }
-    visit_get_next_type(m, (int*) &(*obj)->kind, %(c_name)s_qtypes, name, &err);
+    visit_get_next_type(v, (int*) &(*obj)->kind, %(c_name)s_qtypes, name, &err);
     if (err) {
-        goto out_end;
+        goto out_obj;
     }
     switch ((*obj)->kind) {
 ''',
@@ -218,7 +190,7 @@ void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, Error
     for var in variants.variants:
         ret += mcgen('''
     case %(case)s:
-        visit_type_%(c_type)s(m, &(*obj)->%(c_name)s, name, &err);
+        visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, name, &err);
         break;
 ''',
                      case=c_enum_const(variants.tag_member.type.name,
@@ -230,10 +202,10 @@ void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, Error
     default:
         abort();
     }
-out_end:
+out_obj:
     error_propagate(errp, err);
     err = NULL;
-    visit_end_implicit_struct(m, &err);
+    visit_end_implicit_struct(v, &err);
 out:
     error_propagate(errp, err);
 }
@@ -256,40 +228,40 @@ def gen_visit_union(name, base, variants):
 
     ret += mcgen('''
 
-void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, Error **errp)
+void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error **errp)
 {
     Error *err = NULL;
 
-    visit_start_struct(m, (void **)obj, "%(name)s", name, sizeof(%(c_name)s), &err);
+    visit_start_struct(v, (void **)obj, "%(name)s", name, sizeof(%(c_name)s), &err);
     if (err) {
         goto out;
     }
-    if (*obj) {
+    if (!*obj) {
+        goto out_obj;
+    }
 ''',
                  c_name=c_name(name), name=name)
 
     if base:
         ret += mcgen('''
-        visit_type_%(c_name)s_fields(m, obj, &err);
-        if (err) {
-            goto out_obj;
-        }
+    visit_type_%(c_name)s_fields(v, obj, &err);
 ''',
                      c_name=c_name(name))
+        ret += gen_err_check(label='out_obj')
 
     tag_key = variants.tag_member.name
     if not variants.tag_name:
         # we pointlessly use a different key for simple unions
         tag_key = 'type'
     ret += mcgen('''
-        visit_type_%(c_type)s(m, &(*obj)->%(c_name)s, "%(name)s", &err);
-        if (err) {
-            goto out_obj;
-        }
-        if (!visit_start_union(m, !!(*obj)->data, &err) || err) {
-            goto out_obj;
-        }
-        switch ((*obj)->%(c_name)s) {
+    visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, "%(name)s", &err);
+    if (err) {
+        goto out_obj;
+    }
+    if (!visit_start_union(v, !!(*obj)->data, &err) || err) {
+        goto out_obj;
+    }
+    switch ((*obj)->%(c_name)s) {
 ''',
                  c_type=variants.tag_member.type.c_name(),
                  # TODO ugly special case for simple union
@@ -302,38 +274,37 @@ void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, Error
         # TODO ugly special case for simple union
         simple_union_type = var.simple_union_type()
         ret += mcgen('''
-        case %(case)s:
+    case %(case)s:
 ''',
                      case=c_enum_const(variants.tag_member.type.name,
                                        var.name))
         if simple_union_type:
             ret += mcgen('''
-            visit_type_%(c_type)s(m, &(*obj)->%(c_name)s, "data", &err);
+        visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, "data", &err);
 ''',
                          c_type=simple_union_type.c_name(),
                          c_name=c_name(var.name))
         else:
             ret += mcgen('''
-            visit_type_implicit_%(c_type)s(m, &(*obj)->%(c_name)s, &err);
+        visit_type_implicit_%(c_type)s(v, &(*obj)->%(c_name)s, &err);
 ''',
                          c_type=var.type.c_name(),
                          c_name=c_name(var.name))
         ret += mcgen('''
-            break;
+        break;
 ''')
 
     ret += mcgen('''
-        default:
-            abort();
-        }
-out_obj:
-        error_propagate(errp, err);
-        err = NULL;
-        visit_end_union(m, !!(*obj)->data, &err);
-        error_propagate(errp, err);
-        err = NULL;
+    default:
+        abort();
     }
-    visit_end_struct(m, &err);
+out_obj:
+    error_propagate(errp, err);
+    err = NULL;
+    visit_end_union(v, !!(*obj)->data, &err);
+    error_propagate(errp, err);
+    err = NULL;
+    visit_end_struct(v, &err);
 out:
     error_propagate(errp, err);
 }
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 06478bb269..26cff3f05c 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -71,6 +71,7 @@ all_names = {}
 # Parsing the schema into expressions
 #
 
+
 def error_path(parent):
     res = ""
     while parent:
@@ -79,8 +80,10 @@ def error_path(parent):
         parent = parent['parent']
     return res
 
+
 class QAPISchemaError(Exception):
     def __init__(self, schema, msg):
+        Exception.__init__(self)
         self.fname = schema.fname
         self.msg = msg
         self.col = 1
@@ -96,8 +99,10 @@ class QAPISchemaError(Exception):
         return error_path(self.info) + \
             "%s:%d:%d: %s" % (self.fname, self.line, self.col, self.msg)
 
+
 class QAPIExprError(Exception):
     def __init__(self, expr_info, msg):
+        Exception.__init__(self)
         self.info = expr_info
         self.msg = msg
 
@@ -105,9 +110,10 @@ class QAPIExprError(Exception):
         return error_path(self.info['parent']) + \
             "%s:%d: %s" % (self.info['file'], self.info['line'], self.msg)
 
+
 class QAPISchemaParser(object):
 
-    def __init__(self, fp, previously_included = [], incl_info = None):
+    def __init__(self, fp, previously_included=[], incl_info=None):
         abs_fname = os.path.abspath(fp.name)
         fname = fp.name
         self.fname = fname
@@ -122,18 +128,18 @@ class QAPISchemaParser(object):
         self.exprs = []
         self.accept()
 
-        while self.tok != None:
+        while self.tok is not None:
             expr_info = {'file': fname, 'line': self.line,
                          'parent': self.incl_info}
             expr = self.get_expr(False)
             if isinstance(expr, dict) and "include" in expr:
                 if len(expr) != 1:
-                    raise QAPIExprError(expr_info, "Invalid 'include' directive")
+                    raise QAPIExprError(expr_info,
+                                        "Invalid 'include' directive")
                 include = expr["include"]
                 if not isinstance(include, str):
                     raise QAPIExprError(expr_info,
-                                        'Expected a file name (string), got: %s'
-                                        % include)
+                                        "Value of 'include' must be a string")
                 incl_abs_fname = os.path.join(os.path.dirname(abs_fname),
                                               include)
                 # catch inclusion cycle
@@ -192,7 +198,7 @@ class QAPISchemaParser(object):
                             string += '\t'
                         elif ch == 'u':
                             value = 0
-                            for x in range(0, 4):
+                            for _ in range(0, 4):
                                 ch = self.src[self.cursor]
                                 self.cursor += 1
                                 if ch not in "0123456789abcdefABCDEF":
@@ -214,7 +220,7 @@ class QAPISchemaParser(object):
                             string += ch
                         else:
                             raise QAPISchemaError(self,
-                                                  "Unknown escape \\%s" %ch)
+                                                  "Unknown escape \\%s" % ch)
                         esc = False
                     elif ch == "\\":
                         esc = True
@@ -274,7 +280,7 @@ class QAPISchemaParser(object):
         if self.tok == ']':
             self.accept()
             return expr
-        if not self.tok in "{['tfn":
+        if self.tok not in "{['tfn":
             raise QAPISchemaError(self, 'Expected "{", "[", "]", string, '
                                   'boolean or "null"')
         while True:
@@ -308,15 +314,17 @@ class QAPISchemaParser(object):
 # TODO catching name collisions in generated code would be nice
 #
 
+
 def find_base_fields(base):
     base_struct_define = find_struct(base)
     if not base_struct_define:
         return None
     return base_struct_define['data']
 
+
 # Return the qtype of an alternate branch, or None on error.
 def find_alternate_member_qtype(qapi_type):
-    if builtin_types.has_key(qapi_type):
+    if qapi_type in builtin_types:
         return builtin_types[qapi_type]
     elif find_struct(qapi_type):
         return "QTYPE_QDICT"
@@ -326,6 +334,7 @@ def find_alternate_member_qtype(qapi_type):
         return "QTYPE_QDICT"
     return None
 
+
 # Return the discriminator enum define if discriminator is specified as an
 # enum type, otherwise return None.
 def discriminator_find_enum_define(expr):
@@ -345,11 +354,14 @@ def discriminator_find_enum_define(expr):
 
     return find_enum(discriminator_type)
 
+
 # FIXME should enforce "other than downstream extensions [...], all
 # names should begin with a letter".
 valid_name = re.compile('^[a-zA-Z_][a-zA-Z0-9_.-]*$')
-def check_name(expr_info, source, name, allow_optional = False,
-               enum_member = False):
+
+
+def check_name(expr_info, source, name, allow_optional=False,
+               enum_member=False):
     global valid_name
     membername = name
 
@@ -370,7 +382,8 @@ def check_name(expr_info, source, name, allow_optional = False,
         raise QAPIExprError(expr_info,
                             "%s uses invalid name '%s'" % (source, name))
 
-def add_name(name, info, meta, implicit = False):
+
+def add_name(name, info, meta, implicit=False):
     global all_names
     check_name(info, "'%s'" % meta, name)
     # FIXME should reject names that differ only in '_' vs. '.'
@@ -385,12 +398,14 @@ def add_name(name, info, meta, implicit = False):
                             % (meta, name))
     all_names[name] = meta
 
+
 def add_struct(definition, info):
     global struct_types
     name = definition['struct']
     add_name(name, info, 'struct')
     struct_types.append(definition)
 
+
 def find_struct(name):
     global struct_types
     for struct in struct_types:
@@ -398,12 +413,14 @@ def find_struct(name):
             return struct
     return None
 
+
 def add_union(definition, info):
     global union_types
     name = definition['union']
     add_name(name, info, 'union')
     union_types.append(definition)
 
+
 def find_union(name):
     global union_types
     for union in union_types:
@@ -411,11 +428,13 @@ def find_union(name):
             return union
     return None
 
-def add_enum(name, info, enum_values = None, implicit = False):
+
+def add_enum(name, info, enum_values=None, implicit=False):
     global enum_types
     add_name(name, info, 'enum', implicit)
     enum_types.append({"enum_name": name, "enum_values": enum_values})
 
+
 def find_enum(name):
     global enum_types
     for enum in enum_types:
@@ -423,12 +442,14 @@ def find_enum(name):
             return enum
     return None
 
+
 def is_enum(name):
-    return find_enum(name) != None
+    return find_enum(name) is not None
+
 
-def check_type(expr_info, source, value, allow_array = False,
-               allow_dict = False, allow_optional = False,
-               allow_metas = []):
+def check_type(expr_info, source, value, allow_array=False,
+               allow_dict=False, allow_optional=False,
+               allow_metas=[]):
     global all_names
 
     if value is None:
@@ -447,7 +468,7 @@ def check_type(expr_info, source, value, allow_array = False,
 
     # Check if type name for value is okay
     if isinstance(value, str):
-        if not value in all_names:
+        if value not in all_names:
             raise QAPIExprError(expr_info,
                                 "%s uses unknown type '%s'"
                                 % (source, value))
@@ -476,7 +497,8 @@ def check_type(expr_info, source, value, allow_array = False,
                    allow_metas=['built-in', 'union', 'alternate', 'struct',
                                 'enum'])
 
-def check_member_clash(expr_info, base_name, data, source = ""):
+
+def check_member_clash(expr_info, base_name, data, source=""):
     base = find_struct(base_name)
     assert base
     base_members = base['data']
@@ -490,6 +512,7 @@ def check_member_clash(expr_info, base_name, data, source = ""):
     if base.get('base'):
         check_member_clash(expr_info, base['base'], data, source)
 
+
 def check_command(expr, expr_info):
     name = expr['command']
 
@@ -503,6 +526,7 @@ def check_command(expr, expr_info):
                expr.get('returns'), allow_array=True,
                allow_optional=True, allow_metas=returns_meta)
 
+
 def check_event(expr, expr_info):
     global events
     name = expr['event']
@@ -514,19 +538,20 @@ def check_event(expr, expr_info):
                expr.get('data'), allow_dict=True, allow_optional=True,
                allow_metas=['struct'])
 
+
 def check_union(expr, expr_info):
     name = expr['union']
     base = expr.get('base')
     discriminator = expr.get('discriminator')
     members = expr['data']
-    values = { 'MAX': '(automatic)' }
+    values = {'MAX': '(automatic)', 'KIND': '(automatic)'}
 
     # Two types of unions, determined by discriminator.
 
     # With no discriminator it is a simple union.
     if discriminator is None:
         enum_define = None
-        allow_metas=['built-in', 'union', 'alternate', 'struct', 'enum']
+        allow_metas = ['built-in', 'union', 'alternate', 'struct', 'enum']
         if base is not None:
             raise QAPIExprError(expr_info,
                                 "Simple union '%s' must not have a base"
@@ -535,15 +560,14 @@ def check_union(expr, expr_info):
     # Else, it's a flat union.
     else:
         # The object must have a string member 'base'.
-        if not isinstance(base, str):
+        check_type(expr_info, "'base' for union '%s'" % name,
+                   base, allow_metas=['struct'])
+        if not base:
             raise QAPIExprError(expr_info,
-                                "Flat union '%s' must have a string base field"
+                                "Flat union '%s' must have a base"
                                 % name)
         base_fields = find_base_fields(base)
-        if not base_fields:
-            raise QAPIExprError(expr_info,
-                                "Base '%s' is not a valid struct"
-                                % base)
+        assert base_fields
 
         # The value of member 'discriminator' must name a non-optional
         # member of the base struct.
@@ -556,7 +580,7 @@ def check_union(expr, expr_info):
                                 "struct '%s'"
                                 % (discriminator, base))
         enum_define = find_enum(discriminator_type)
-        allow_metas=['struct']
+        allow_metas = ['struct']
         # Do not allow string discriminator
         if not enum_define:
             raise QAPIExprError(expr_info,
@@ -578,13 +602,19 @@ def check_union(expr, expr_info):
                                " of branch '%s'" % key)
 
         # If the discriminator names an enum type, then all members
-        # of 'data' must also be members of the enum type.
+        # of 'data' must also be members of the enum type, which in turn
+        # must not collide with the discriminator name.
         if enum_define:
-            if not key in enum_define['enum_values']:
+            if key not in enum_define['enum_values']:
                 raise QAPIExprError(expr_info,
                                     "Discriminator value '%s' is not found in "
                                     "enum '%s'" %
                                     (key, enum_define["enum_name"]))
+            if discriminator in enum_define['enum_values']:
+                raise QAPIExprError(expr_info,
+                                    "Discriminator name '%s' collides with "
+                                    "enum value in '%s'" %
+                                    (discriminator, enum_define["enum_name"]))
 
         # Otherwise, check for conflicts in the generated enum
         else:
@@ -595,10 +625,11 @@ def check_union(expr, expr_info):
                                     % (name, key, values[c_key]))
             values[c_key] = key
 
+
 def check_alternate(expr, expr_info):
     name = expr['alternate']
     members = expr['data']
-    values = { 'MAX': '(automatic)' }
+    values = {'MAX': '(automatic)'}
     types_seen = {}
 
     # Check every branch
@@ -626,11 +657,12 @@ def check_alternate(expr, expr_info):
                                 % (name, key, types_seen[qtype]))
         types_seen[qtype] = key
 
+
 def check_enum(expr, expr_info):
     name = expr['enum']
     members = expr.get('data')
     prefix = expr.get('prefix')
-    values = { 'MAX': '(automatic)' }
+    values = {'MAX': '(automatic)'}
 
     if not isinstance(members, list):
         raise QAPIExprError(expr_info,
@@ -639,7 +671,7 @@ def check_enum(expr, expr_info):
         raise QAPIExprError(expr_info,
                             "Enum '%s' requires a string for 'prefix'" % name)
     for member in members:
-        check_name(expr_info, "Member of enum '%s'" %name, member,
+        check_name(expr_info, "Member of enum '%s'" % name, member,
                    enum_member=True)
         key = camel_to_upper(member)
         if key in values:
@@ -648,6 +680,7 @@ def check_enum(expr, expr_info):
                                 % (name, member, values[key]))
         values[key] = member
 
+
 def check_struct(expr, expr_info):
     name = expr['struct']
     members = expr['data']
@@ -659,6 +692,7 @@ def check_struct(expr, expr_info):
     if expr.get('base'):
         check_member_clash(expr_info, expr['base'], expr['data'])
 
+
 def check_keys(expr_elem, meta, required, optional=[]):
     expr = expr_elem['expr']
     info = expr_elem['info']
@@ -666,22 +700,23 @@ def check_keys(expr_elem, meta, required, optional=[]):
     if not isinstance(name, str):
         raise QAPIExprError(info,
                             "'%s' key must have a string value" % meta)
-    required = required + [ meta ]
+    required = required + [meta]
     for (key, value) in expr.items():
-        if not key in required and not key in optional:
+        if key not in required and key not in optional:
             raise QAPIExprError(info,
                                 "Unknown key '%s' in %s '%s'"
                                 % (key, meta, name))
-        if (key == 'gen' or key == 'success-response') and value != False:
+        if (key == 'gen' or key == 'success-response') and value is not False:
             raise QAPIExprError(info,
                                 "'%s' of %s '%s' should only use false value"
                                 % (key, meta, name))
     for key in required:
-        if not expr.has_key(key):
+        if key not in expr:
             raise QAPIExprError(info,
                                 "Key '%s' is missing from %s '%s'"
                                 % (key, meta, name))
 
+
 def check_exprs(exprs):
     global all_names
 
@@ -691,24 +726,24 @@ def check_exprs(exprs):
     for expr_elem in exprs:
         expr = expr_elem['expr']
         info = expr_elem['info']
-        if expr.has_key('enum'):
+        if 'enum' in expr:
             check_keys(expr_elem, 'enum', ['data'], ['prefix'])
             add_enum(expr['enum'], info, expr['data'])
-        elif expr.has_key('union'):
+        elif 'union' in expr:
             check_keys(expr_elem, 'union', ['data'],
                        ['base', 'discriminator'])
             add_union(expr, info)
-        elif expr.has_key('alternate'):
+        elif 'alternate' in expr:
             check_keys(expr_elem, 'alternate', ['data'])
             add_name(expr['alternate'], info, 'alternate')
-        elif expr.has_key('struct'):
+        elif 'struct' in expr:
             check_keys(expr_elem, 'struct', ['data'], ['base'])
             add_struct(expr, info)
-        elif expr.has_key('command'):
+        elif 'command' in expr:
             check_keys(expr_elem, 'command', [],
                        ['data', 'returns', 'gen', 'success-response'])
             add_name(expr['command'], info, 'command')
-        elif expr.has_key('event'):
+        elif 'event' in expr:
             check_keys(expr_elem, 'event', [], ['data'])
             add_name(expr['event'], info, 'event')
         else:
@@ -718,11 +753,11 @@ def check_exprs(exprs):
     # Try again for hidden UnionKind enum
     for expr_elem in exprs:
         expr = expr_elem['expr']
-        if expr.has_key('union'):
+        if 'union' in expr:
             if not discriminator_find_enum_define(expr):
                 add_enum('%sKind' % expr['union'], expr_elem['info'],
                          implicit=True)
-        elif expr.has_key('alternate'):
+        elif 'alternate' in expr:
             add_enum('%sKind' % expr['alternate'], expr_elem['info'],
                      implicit=True)
 
@@ -731,17 +766,17 @@ def check_exprs(exprs):
         expr = expr_elem['expr']
         info = expr_elem['info']
 
-        if expr.has_key('enum'):
+        if 'enum' in expr:
             check_enum(expr, info)
-        elif expr.has_key('union'):
+        elif 'union' in expr:
             check_union(expr, info)
-        elif expr.has_key('alternate'):
+        elif 'alternate' in expr:
             check_alternate(expr, info)
-        elif expr.has_key('struct'):
+        elif 'struct' in expr:
             check_struct(expr, info)
-        elif expr.has_key('command'):
+        elif 'command' in expr:
             check_command(expr, info)
-        elif expr.has_key('event'):
+        elif 'event' in expr:
             check_event(expr, info)
         else:
             assert False, 'unexpected meta type'
@@ -993,6 +1028,7 @@ class QAPISchemaObjectTypeVariants(object):
             vseen = dict(seen)
             v.check(schema, self.tag_member.type, vseen)
 
+
 class QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember):
     def __init__(self, name, typ):
         QAPISchemaObjectTypeMember.__init__(self, name, typ, False)
@@ -1292,6 +1328,7 @@ def camel_case(name):
             new_name += ch.lower()
     return new_name
 
+
 # ENUMName -> ENUM_NAME, EnumName1 -> ENUM_NAME1
 # ENUM_NAME -> ENUM_NAME, ENUM_NAME1 -> ENUM_NAME1, ENUM_Name2 -> ENUM_NAME2
 # ENUM24_Name -> ENUM24_NAME
@@ -1306,14 +1343,14 @@ def camel_to_upper(value):
         c = c_fun_str[i]
         # When c is upper and no "_" appears before, do more checks
         if c.isupper() and (i > 0) and c_fun_str[i - 1] != "_":
-            # Case 1: next string is lower
-            # Case 2: previous string is digit
-            if (i < (l - 1) and c_fun_str[i + 1].islower()) or \
-            c_fun_str[i - 1].isdigit():
+            if i < l - 1 and c_fun_str[i + 1].islower():
+                new_name += '_'
+            elif c_fun_str[i - 1].isdigit():
                 new_name += '_'
         new_name += c
     return new_name.lstrip('_').upper()
 
+
 def c_enum_const(type_name, const_name, prefix=None):
     if prefix is not None:
         type_name = prefix
@@ -1321,6 +1358,7 @@ def c_enum_const(type_name, const_name, prefix=None):
 
 c_name_trans = string.maketrans('.-', '__')
 
+
 # Map @name to a valid C identifier.
 # If @protect, avoid returning certain ticklish identifiers (like
 # C keywords) by prepending "q_".
@@ -1333,15 +1371,16 @@ c_name_trans = string.maketrans('.-', '__')
 def c_name(name, protect=True):
     # ANSI X3J11/88-090, 3.1.1
     c89_words = set(['auto', 'break', 'case', 'char', 'const', 'continue',
-                     'default', 'do', 'double', 'else', 'enum', 'extern', 'float',
-                     'for', 'goto', 'if', 'int', 'long', 'register', 'return',
-                     'short', 'signed', 'sizeof', 'static', 'struct', 'switch',
-                     'typedef', 'union', 'unsigned', 'void', 'volatile', 'while'])
+                     'default', 'do', 'double', 'else', 'enum', 'extern',
+                     'float', 'for', 'goto', 'if', 'int', 'long', 'register',
+                     'return', 'short', 'signed', 'sizeof', 'static',
+                     'struct', 'switch', 'typedef', 'union', 'unsigned',
+                     'void', 'volatile', 'while'])
     # ISO/IEC 9899:1999, 6.4.1
     c99_words = set(['inline', 'restrict', '_Bool', '_Complex', '_Imaginary'])
     # ISO/IEC 9899:2011, 6.4.1
-    c11_words = set(['_Alignas', '_Alignof', '_Atomic', '_Generic', '_Noreturn',
-                     '_Static_assert', '_Thread_local'])
+    c11_words = set(['_Alignas', '_Alignof', '_Atomic', '_Generic',
+                     '_Noreturn', '_Static_assert', '_Thread_local'])
     # GCC http://gcc.gnu.org/onlinedocs/gcc-4.7.1/gcc/C-Extensions.html
     # excluding _.*
     gcc_words = set(['asm', 'typeof'])
@@ -1357,29 +1396,34 @@ def c_name(name, protect=True):
                      'not_eq', 'or', 'or_eq', 'xor', 'xor_eq'])
     # namespace pollution:
     polluted_words = set(['unix', 'errno'])
-    if protect and (name in c89_words | c99_words | c11_words | gcc_words | cpp_words | polluted_words):
+    if protect and (name in c89_words | c99_words | c11_words | gcc_words
+                    | cpp_words | polluted_words):
         return "q_" + name
     return name.translate(c_name_trans)
 
 eatspace = '\033EATSPACE.'
 pointer_suffix = ' *' + eatspace
 
+
 def genindent(count):
     ret = ""
-    for i in range(count):
+    for _ in range(count):
         ret += " "
     return ret
 
 indent_level = 0
 
+
 def push_indent(indent_amount=4):
     global indent_level
     indent_level += indent_amount
 
+
 def pop_indent(indent_amount=4):
     global indent_level
     indent_level -= indent_amount
 
+
 # Generate @code with @kwds interpolated.
 # Obey indent_level, and strip eatspace.
 def cgen(code, **kwds):
@@ -1392,6 +1436,7 @@ def cgen(code, **kwds):
         raw = raw[0]
     return re.sub(re.escape(eatspace) + ' *', '', raw)
 
+
 def mcgen(code, **kwds):
     if code[0] == '\n':
         code = code[1:]
@@ -1401,6 +1446,7 @@ def mcgen(code, **kwds):
 def guardname(filename):
     return c_name(filename, protect=False).upper()
 
+
 def guardstart(name):
     return mcgen('''
 
@@ -1410,6 +1456,7 @@ def guardstart(name):
 ''',
                  name=guardname(name))
 
+
 def guardend(name):
     return mcgen('''
 
@@ -1418,6 +1465,7 @@ def guardend(name):
 ''',
                  name=guardname(name))
 
+
 def gen_enum_lookup(name, values, prefix=None):
     ret = mcgen('''
 
@@ -1439,6 +1487,7 @@ const char *const %(c_name)s_lookup[] = {
                  max_index=max_index)
     return ret
 
+
 def gen_enum(name, values, prefix=None):
     # append automatically generated _MAX value
     enum_values = values + ['MAX']
@@ -1470,6 +1519,7 @@ extern const char *const %(c_name)s_lookup[];
                  c_name=c_name(name))
     return ret
 
+
 def gen_params(arg_type, extra):
     if not arg_type:
         return extra
@@ -1486,11 +1536,67 @@ def gen_params(arg_type, extra):
         ret += sep + extra
     return ret
 
+
+def gen_err_check(label='out', skiperr=False):
+    if skiperr:
+        return ''
+    return mcgen('''
+    if (err) {
+        goto %(label)s;
+    }
+''',
+                 label=label)
+
+
+def gen_visit_fields(members, prefix='', need_cast=False, skiperr=False):
+    ret = ''
+    if skiperr:
+        errparg = 'NULL'
+    else:
+        errparg = '&err'
+
+    for memb in members:
+        if memb.optional:
+            ret += mcgen('''
+    visit_optional(v, &%(prefix)shas_%(c_name)s, "%(name)s", %(errp)s);
+''',
+                         prefix=prefix, c_name=c_name(memb.name),
+                         name=memb.name, errp=errparg)
+            ret += gen_err_check(skiperr=skiperr)
+            ret += mcgen('''
+    if (%(prefix)shas_%(c_name)s) {
+''',
+                         prefix=prefix, c_name=c_name(memb.name))
+            push_indent()
+
+        # Ugly: sometimes we need to cast away const
+        if need_cast and memb.type.name == 'str':
+            cast = '(char **)'
+        else:
+            cast = ''
+
+        ret += mcgen('''
+    visit_type_%(c_type)s(v, %(cast)s&%(prefix)s%(c_name)s, "%(name)s", %(errp)s);
+''',
+                     c_type=memb.type.c_name(), prefix=prefix, cast=cast,
+                     c_name=c_name(memb.name), name=memb.name,
+                     errp=errparg)
+        ret += gen_err_check(skiperr=skiperr)
+
+        if memb.optional:
+            pop_indent()
+            ret += mcgen('''
+    }
+''')
+    return ret
+
+
 #
 # Common command line parsing
 #
 
-def parse_command_line(extra_options = "", extra_long_options = []):
+
+def parse_command_line(extra_options="", extra_long_options=[]):
 
     try:
         opts, args = getopt.gnu_getopt(sys.argv[1:],
@@ -1541,6 +1647,7 @@ def parse_command_line(extra_options = "", extra_long_options = []):
 # Generate output files with boilerplate
 #
 
+
 def open_output(output_dir, do_c, do_h, prefix, c_file, h_file,
                 c_comment, h_comment):
     guard = guardname(prefix + h_file)
@@ -1568,7 +1675,7 @@ def open_output(output_dir, do_c, do_h, prefix, c_file, h_file,
 /* AUTOMATICALLY GENERATED, DO NOT MODIFY */
 %(comment)s
 ''',
-                     comment = c_comment))
+                     comment=c_comment))
 
     fdecl.write(mcgen('''
 /* AUTOMATICALLY GENERATED, DO NOT MODIFY */
@@ -1577,10 +1684,11 @@ def open_output(output_dir, do_c, do_h, prefix, c_file, h_file,
 #define %(guard)s
 
 ''',
-                      comment = h_comment, guard = guard))
+                      comment=h_comment, guard=guard))
 
     return (fdef, fdecl)
 
+
 def close_output(fdef, fdecl):
     fdecl.write('''
 #endif