From 4247f839009159cb2cbaddfbd41513e180c4fe52 Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Tue, 9 Jun 2015 15:24:36 +0200
Subject: [PATCH 01/33] qapi: Clarify docs on including the same file multiple
 times

It's idempotent.

While there, update examples to current code.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 docs/qapi-code-gen.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index 61b5be47fb..e7e7281f5b 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -163,7 +163,7 @@ The QAPI schema definitions can be modularized using the 'include' directive:
 
 The directive is evaluated recursively, and include paths are relative to the
 file using the directive. Multiple includes of the same file are
-safe.  No other keys should appear in the expression, and the include
+idempotent.  No other keys should appear in the expression, and the include
 value should be a string.
 
 As a matter of style, it is a good idea to have all files be
@@ -555,6 +555,7 @@ Example:
         qapi_dealloc_visitor_cleanup(md);
     }
 
+
     void qapi_free_UserDefOne(UserDefOne *obj)
     {
         QapiDeallocVisitor *md;
@@ -769,7 +770,6 @@ Example:
         v = qapi_dealloc_get_visitor(md);
         visit_type_UserDefOne(v, &arg1, "arg1", NULL);
         qapi_dealloc_visitor_cleanup(md);
-        return;
     }
 
     static void qmp_init_marshal(void)

From 77e703b861d34bb2879f3e845482d5cf0a3a0ad1 Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Wed, 24 Jun 2015 19:27:32 +0200
Subject: [PATCH 02/33] qapi: Clean up cgen() and mcgen()

Commit 05dfb26 added eatspace stripping to mcgen().  Move it to
cgen(), just in case somebody gets tempted to use cgen() directly
instead of via mcgen().

cgen() indents blank lines.  No such lines get generated right now,
but fix it anyway.

We use triple-quoted strings for program text, like this:

    '''
    Program text
    any number of lines
    '''

Keeps the program text relatively readable, but puts an extra newline
at either end.  mcgen() "fixes" that by dropping the first and last
line outright.  Drop only the newlines.

This unmasks a bug in qapi-commands.py: four quotes instead of three.
Fix it up.

Output doesn't change

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 scripts/qapi-commands.py |  2 +-
 scripts/qapi.py          | 17 +++++++++++------
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index ca22acc1d5..ce5140865b 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -56,7 +56,7 @@ def gen_sync_call(name, args, ret_type, indent=0):
                 name=c_name(name), args=arglist, retval=retval).rstrip()
     if ret_type:
         ret += "\n" + gen_err_check('local_err')
-        ret += "\n" + mcgen(''''
+        ret += "\n" + mcgen('''
 %(marshal_output_call)s
 ''',
                             marshal_output_call=gen_marshal_output_call(name, ret_type)).rstrip()
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 06d7fc2848..e656beb58b 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -943,15 +943,20 @@ 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):
-    indent = genindent(indent_level)
-    lines = code.split('\n')
-    lines = map(lambda x: indent + x, lines)
-    return '\n'.join(lines) % kwds + '\n'
+    raw = code % kwds
+    if indent_level:
+        indent = genindent(indent_level)
+        raw = re.subn("^.", indent + r'\g<0>', raw, 0, re.MULTILINE)
+        raw = raw[0]
+    return re.sub(re.escape(eatspace) + ' *', '', raw)
 
 def mcgen(code, **kwds):
-    raw = cgen('\n'.join(code.split('\n')[1:-1]), **kwds)
-    return re.sub(re.escape(eatspace) + ' *', '', raw)
+    if code[0] == '\n':
+        code = code[1:]
+    return cgen(code, **kwds)
 
 def basename(filename):
     return filename.split("/")[-1]

From 00dfc3b2c272d98556ec6095d56bdd8b036babf9 Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Sat, 27 Jun 2015 07:27:21 +0200
Subject: [PATCH 03/33] qapi: Simplify guardname()

The guards around built-in declarations lose their _H.  It never made
much sense anyway.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 scripts/qapi.py | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index e656beb58b..ba11c54a92 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -958,14 +958,9 @@ def mcgen(code, **kwds):
         code = code[1:]
     return cgen(code, **kwds)
 
-def basename(filename):
-    return filename.split("/")[-1]
 
 def guardname(filename):
-    guard = basename(filename).rsplit(".", 1)[0]
-    for substr in [".", " ", "-"]:
-        guard = guard.replace(substr, "_")
-    return guard.upper() + '_H'
+    return c_name(filename, protect=False).upper()
 
 def guardstart(name):
     return mcgen('''
@@ -1035,6 +1030,7 @@ def parse_command_line(extra_options = "", extra_long_options = []):
 
 def open_output(output_dir, do_c, do_h, prefix, c_file, h_file,
                 c_comment, h_comment):
+    guard = guardname(prefix + h_file)
     c_file = output_dir + prefix + c_file
     h_file = output_dir + prefix + h_file
 
@@ -1067,7 +1063,7 @@ def open_output(output_dir, do_c, do_h, prefix, c_file, h_file,
 #define %(guard)s
 
 ''',
-                      comment = h_comment, guard = guardname(h_file)))
+                      comment = h_comment, guard = guard))
 
     return (fdef, fdecl)
 

From 016a335bd8ca624f43adbb08fa1698c29ec52a1a Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Wed, 1 Jul 2015 12:59:40 +0200
Subject: [PATCH 04/33] qapi-event: Clean up how name of enum QAPIEvent is made

Use c_name() instead of ad hoc code.  Doesn't upcase the -p prefix,
which is an improvement in my book.  Unbreaks prefix containing '.',
but other funny characters remain broken.  To be fixed next.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 docs/qapi-code-gen.txt | 8 ++++----
 scripts/qapi-event.py  | 2 +-
 tests/test-qmp-event.c | 2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index e7e7281f5b..c2ac21c889 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -826,7 +826,7 @@ Example:
         QDECREF(qmp);
     }
 
-    const char *EXAMPLE_QAPIEvent_lookup[] = {
+    const char *example_QAPIEvent_lookup[] = {
         "MY_EVENT",
         NULL,
     };
@@ -843,11 +843,11 @@ Example:
 
     void qapi_event_send_my_event(Error **errp);
 
-    extern const char *EXAMPLE_QAPIEvent_lookup[];
-    typedef enum EXAMPLE_QAPIEvent
+    extern const char *example_QAPIEvent_lookup[];
+    typedef enum example_QAPIEvent
     {
         EXAMPLE_QAPI_EVENT_MY_EVENT = 0,
         EXAMPLE_QAPI_EVENT_MAX = 1,
-    } EXAMPLE_QAPIEvent;
+    } example_QAPIEvent;
 
     #endif
diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
index 56bc602a6d..cc74f4dc22 100644
--- a/scripts/qapi-event.py
+++ b/scripts/qapi-event.py
@@ -267,7 +267,7 @@ fdecl.write(mcgen('''
 
 exprs = parse_schema(input_file)
 
-event_enum_name = prefix.upper().replace('-', '_') + "QAPIEvent"
+event_enum_name = c_name(prefix + "QAPIEvent", protect=False)
 event_enum_values = []
 event_enum_strings = []
 
diff --git a/tests/test-qmp-event.c b/tests/test-qmp-event.c
index 1ee40e148a..28f146d4b7 100644
--- a/tests/test-qmp-event.c
+++ b/tests/test-qmp-event.c
@@ -94,7 +94,7 @@ static bool qdict_cmp_simple(QDict *a, QDict *b)
 
 /* This function is hooked as final emit function, which can verify the
    correctness. */
-static void event_test_emit(TEST_QAPIEvent event, QDict *d, Error **errp)
+static void event_test_emit(test_QAPIEvent event, QDict *d, Error **errp)
 {
     QObject *obj;
     QDict *t;

From 1cf47a15f18312436c7fa2d97be5fbe6df0292f5 Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Wed, 1 Jul 2015 13:13:54 +0200
Subject: [PATCH 05/33] qapi: Reject -p arguments that break qapi-event.py

qapi-event.py breaks when you ask for a funny prefix like '@'.
Protect it.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 scripts/qapi.py | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index ba11c54a92..bc3f4d3164 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -1003,6 +1003,12 @@ def parse_command_line(extra_options = "", extra_long_options = []):
     for oa in opts:
         o, a = oa
         if o in ("-p", "--prefix"):
+            match = re.match('([A-Za-z_.-][A-Za-z0-9_.-]*)?', a)
+            if match.end() != len(a):
+                print >>sys.stderr, \
+                    "%s: 'funny character '%s' in argument of --prefix" \
+                    % (sys.argv[0], a[match.end()])
+                sys.exit(1)
             prefix = a
         elif o in ("-o", "--output-dir"):
             output_dir = a + "/"

From 5aa05d3f72e556752167f7005d6a3dea0f4432c5 Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Sun, 28 Jun 2015 21:36:26 +0200
Subject: [PATCH 06/33] qapi: Drop unused and useless parameters and variables

gen_sync_call()'s parameter indent is useless: gen_sync_call() uses it
only as optional argument for push_indent() and pop_indent(), their
default is four, and gen_sync_call()'s only caller passes four.  Drop
the parameter.

gen_visitor_input_containers_decl()'s parameter obj is always
"QOBJECT(args)".  Use that, and drop the parameter.

Drop unused parameters of gen_marshal_output(),
gen_marshal_input_decl(), generate_visit_struct_body(),
generate_visit_list(), generate_visit_enum(), generate_declaration(),
generate_enum_declaration(), generate_decl_enum().

Drop unused variables in generate_event_enum_lookup(),
generate_enum_lookup(), generate_visit_struct_fields(), check_event().

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 scripts/qapi-commands.py | 27 +++++++++++------------
 scripts/qapi-event.py    |  1 -
 scripts/qapi-types.py    |  1 -
 scripts/qapi-visit.py    | 47 +++++++++++++++++++---------------------
 scripts/qapi.py          |  1 -
 5 files changed, 35 insertions(+), 42 deletions(-)

diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index ce5140865b..69029f58fb 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -38,7 +38,7 @@ if (local_err) {
 ''')
     return ''
 
-def gen_sync_call(name, args, ret_type, indent=0):
+def gen_sync_call(name, args, ret_type):
     ret = ""
     arglist=""
     retval=""
@@ -48,7 +48,7 @@ def gen_sync_call(name, args, ret_type, indent=0):
         if optional:
             arglist += "has_%s, " % c_name(argname)
         arglist += "%s, " % (c_name(argname))
-    push_indent(indent)
+    push_indent()
     ret = mcgen('''
 %(retval)sqmp_%(name)s(%(args)s&local_err);
 
@@ -60,7 +60,7 @@ def gen_sync_call(name, args, ret_type, indent=0):
 %(marshal_output_call)s
 ''',
                             marshal_output_call=gen_marshal_output_call(name, ret_type)).rstrip()
-    pop_indent(indent)
+    pop_indent()
     return ret.rstrip()
 
 
@@ -69,17 +69,16 @@ def gen_marshal_output_call(name, ret_type):
         return ""
     return "qmp_marshal_output_%s(retval, ret, &local_err);" % c_name(name)
 
-def gen_visitor_input_containers_decl(args, obj):
+def gen_visitor_input_containers_decl(args):
     ret = ""
 
     push_indent()
     if len(args) > 0:
         ret += mcgen('''
-QmpInputVisitor *mi = qmp_input_visitor_new_strict(%(obj)s);
+QmpInputVisitor *mi = qmp_input_visitor_new_strict(QOBJECT(args));
 QapiDeallocVisitor *md;
 Visitor *v;
-''',
-                     obj=obj)
+''')
     pop_indent()
 
     return ret.rstrip()
@@ -161,7 +160,7 @@ qapi_dealloc_visitor_cleanup(md);
     pop_indent()
     return ret.rstrip()
 
-def gen_marshal_output(name, args, ret_type, middle_mode):
+def gen_marshal_output(name, ret_type):
     if not ret_type:
         return ""
 
@@ -194,14 +193,14 @@ out:
 
     return ret
 
-def gen_marshal_input_decl(name, args, ret_type, middle_mode):
+def gen_marshal_input_decl(name, middle_mode):
     ret = 'void qmp_marshal_input_%s(QDict *args, QObject **ret, Error **errp)' % c_name(name)
     if not middle_mode:
         ret = "static " + ret
     return ret
 
 def gen_marshal_input(name, args, ret_type, middle_mode):
-    hdr = gen_marshal_input_decl(name, args, ret_type, middle_mode)
+    hdr = gen_marshal_input_decl(name, middle_mode)
 
     ret = mcgen('''
 %(header)s
@@ -228,7 +227,7 @@ def gen_marshal_input(name, args, ret_type, middle_mode):
 %(visitor_input_block)s
 
 ''',
-                     visitor_input_containers_decl=gen_visitor_input_containers_decl(args, "QOBJECT(args)"),
+                     visitor_input_containers_decl=gen_visitor_input_containers_decl(args),
                      visitor_input_vars_decl=gen_visitor_input_vars_decl(args),
                      visitor_input_block=gen_visitor_input_block(args))
     else:
@@ -240,7 +239,7 @@ def gen_marshal_input(name, args, ret_type, middle_mode):
     ret += mcgen('''
 %(sync_call)s
 ''',
-                 sync_call=gen_sync_call(name, args, ret_type, indent=4))
+                 sync_call=gen_sync_call(name, args, ret_type))
     if re.search('^ *goto out\\;', ret, re.MULTILINE):
         ret += mcgen('''
 
@@ -360,11 +359,11 @@ for cmd in commands:
     ret = generate_command_decl(cmd['command'], arglist, ret_type) + "\n"
     fdecl.write(ret)
     if ret_type:
-        ret = gen_marshal_output(cmd['command'], arglist, ret_type, middle_mode) + "\n"
+        ret = gen_marshal_output(cmd['command'], ret_type) + "\n"
         fdef.write(ret)
 
     if middle_mode:
-        fdecl.write('%s;\n' % gen_marshal_input_decl(cmd['command'], arglist, ret_type, middle_mode))
+        fdecl.write('%s;\n' % gen_marshal_input_decl(cmd['command'], middle_mode))
 
     ret = gen_marshal_input(cmd['command'], arglist, ret_type, middle_mode) + "\n"
     fdef.write(ret)
diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
index cc74f4dc22..88b0620d00 100644
--- a/scripts/qapi-event.py
+++ b/scripts/qapi-event.py
@@ -199,7 +199,6 @@ const char *%(event_enum_name)s_lookup[] = {
 ''',
                 event_enum_name = event_enum_name)
 
-    i = 0
     for string in event_enum_strings:
         ret += mcgen('''
     "%(string)s",
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index e6eb4b613a..4902440ce3 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -108,7 +108,6 @@ def generate_enum_lookup(name, values):
 const char * const %(name)s_lookup[] = {
 ''',
                 name=c_name(name))
-    i = 0
     for value in values:
         index = c_enum_const(name, value)
         ret += mcgen('''
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 5b99336488..e8ee2688e6 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -40,7 +40,6 @@ static void visit_type_implicit_%(c_type)s(Visitor *m, %(c_type)s **obj, Error *
                  c_type=type_name(type))
 
 def generate_visit_struct_fields(name, members, base = None):
-    substructs = []
     ret = ''
 
     if base:
@@ -103,7 +102,7 @@ out:
     return ret
 
 
-def generate_visit_struct_body(name, members):
+def generate_visit_struct_body(name):
     ret = mcgen('''
     Error *err = NULL;
 
@@ -135,14 +134,14 @@ void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error **e
 ''',
                  name=c_name(name))
 
-    ret += generate_visit_struct_body(name, members)
+    ret += generate_visit_struct_body(name)
 
     ret += mcgen('''
 }
 ''')
     return ret
 
-def generate_visit_list(name, members):
+def generate_visit_list(name):
     return mcgen('''
 
 void visit_type_%(name)sList(Visitor *m, %(name)sList **obj, const char *name, Error **errp)
@@ -171,7 +170,7 @@ out:
 ''',
                 name=type_name(name))
 
-def generate_visit_enum(name, members):
+def generate_visit_enum(name):
     return mcgen('''
 
 void visit_type_%(name)s(Visitor *m, %(name)s *obj, const char *name, Error **errp)
@@ -252,7 +251,7 @@ def generate_visit_union(expr):
     else:
         # There will always be a discriminator in the C switch code, by default
         # it is an enum type generated silently
-        ret = generate_visit_enum(name + 'Kind', members.keys())
+        ret = generate_visit_enum(name + 'Kind')
         disc_type = c_name(name) + 'Kind'
 
     if base:
@@ -340,7 +339,7 @@ out:
 
     return ret
 
-def generate_declaration(name, members, builtin_type=False):
+def generate_declaration(name, builtin_type=False):
     ret = ""
     if not builtin_type:
         name = c_name(name)
@@ -357,7 +356,7 @@ void visit_type_%(name)sList(Visitor *m, %(name)sList **obj, const char *name, E
 
     return ret
 
-def generate_enum_declaration(name, members):
+def generate_enum_declaration(name):
     ret = mcgen('''
 void visit_type_%(name)sList(Visitor *m, %(name)sList **obj, const char *name, Error **errp);
 ''',
@@ -365,7 +364,7 @@ void visit_type_%(name)sList(Visitor *m, %(name)sList **obj, const char *name, E
 
     return ret
 
-def generate_decl_enum(name, members):
+def generate_decl_enum(name):
     return mcgen('''
 
 void visit_type_%(name)s(Visitor *m, %(name)s *obj, const char *name, Error **errp);
@@ -433,7 +432,7 @@ exprs = parse_schema(input_file)
 # for built-in types in our header files and simply guard them
 fdecl.write(guardstart("QAPI_VISIT_BUILTIN_VISITOR_DECL"))
 for typename in builtin_types.keys():
-    fdecl.write(generate_declaration(typename, None, builtin_type=True))
+    fdecl.write(generate_declaration(typename, builtin_type=True))
 fdecl.write(guardend("QAPI_VISIT_BUILTIN_VISITOR_DECL"))
 
 # ...this doesn't work for cases where we link in multiple objects that
@@ -441,44 +440,42 @@ fdecl.write(guardend("QAPI_VISIT_BUILTIN_VISITOR_DECL"))
 # over these cases
 if do_builtins:
     for typename in builtin_types.keys():
-        fdef.write(generate_visit_list(typename, None))
+        fdef.write(generate_visit_list(typename))
 
 for expr in exprs:
     if expr.has_key('struct'):
         ret = generate_visit_struct(expr)
-        ret += generate_visit_list(expr['struct'], expr['data'])
+        ret += generate_visit_list(expr['struct'])
         fdef.write(ret)
 
-        ret = generate_declaration(expr['struct'], expr['data'])
+        ret = generate_declaration(expr['struct'])
         fdecl.write(ret)
     elif expr.has_key('union'):
         ret = generate_visit_union(expr)
-        ret += generate_visit_list(expr['union'], expr['data'])
+        ret += generate_visit_list(expr['union'])
         fdef.write(ret)
 
         enum_define = discriminator_find_enum_define(expr)
         ret = ""
         if not enum_define:
-            ret = generate_decl_enum('%sKind' % expr['union'],
-                                     expr['data'].keys())
-        ret += generate_declaration(expr['union'], expr['data'])
+            ret = generate_decl_enum('%sKind' % expr['union'])
+        ret += generate_declaration(expr['union'])
         fdecl.write(ret)
     elif expr.has_key('alternate'):
         ret = generate_visit_alternate(expr['alternate'], expr['data'])
-        ret += generate_visit_list(expr['alternate'], expr['data'])
+        ret += generate_visit_list(expr['alternate'])
         fdef.write(ret)
 
-        ret = generate_decl_enum('%sKind' % expr['alternate'],
-                                 expr['data'].keys())
-        ret += generate_declaration(expr['alternate'], expr['data'])
+        ret = generate_decl_enum('%sKind' % expr['alternate'])
+        ret += generate_declaration(expr['alternate'])
         fdecl.write(ret)
     elif expr.has_key('enum'):
-        ret = generate_visit_list(expr['enum'], expr['data'])
-        ret += generate_visit_enum(expr['enum'], expr['data'])
+        ret = generate_visit_list(expr['enum'])
+        ret += generate_visit_enum(expr['enum'])
         fdef.write(ret)
 
-        ret = generate_decl_enum(expr['enum'], expr['data'])
-        ret += generate_enum_declaration(expr['enum'], expr['data'])
+        ret = generate_decl_enum(expr['enum'])
+        ret += generate_enum_declaration(expr['enum'])
         fdecl.write(ret)
 
 close_output(fdef, fdecl)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index bc3f4d3164..e7c814dbc8 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -507,7 +507,6 @@ def check_command(expr, expr_info):
 def check_event(expr, expr_info):
     global events
     name = expr['event']
-    params = expr.get('data')
 
     if name.upper() == 'MAX':
         raise QAPIExprError(expr_info, "Event name 'MAX' cannot be created")

From 0f61af3eb396ae163cd1572ce12e05f5d08d7c15 Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Fri, 31 Jul 2015 10:30:04 +0200
Subject: [PATCH 07/33] qapi: Fix generated code when flat union has member
 'kind'

A flat union's tag member gets renamed to 'kind' in the generated
code.  Breaks when another member named 'kind' exists.

Example, adapted from qapi-schema-test.json:

    { 'struct': 'UserDefUnionBase',
      'data': { 'kind': 'str', 'enum1': 'EnumOne' } }

We generate:

    struct UserDefFlatUnion
    {
        EnumOne kind;
        union {
            void *data;
            UserDefA *value1;
            UserDefB *value2;
            UserDefB *value3;
        };
        char *kind;
    };

Kill the silly rename.

Reported-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 scripts/qapi-types.py           | 3 ++-
 scripts/qapi-visit.py           | 7 +++++--
 tests/test-qmp-input-visitor.c  | 2 +-
 tests/test-qmp-output-visitor.c | 2 +-
 4 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 4902440ce3..ac8dad3171 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -200,11 +200,12 @@ def generate_union(expr, meta):
     ret = mcgen('''
 struct %(name)s
 {
-    %(discriminator_type_name)s kind;
+    %(discriminator_type_name)s %(discriminator)s;
     union {
         void *data;
 ''',
                 name=name,
+                discriminator=c_name(discriminator or 'kind'),
                 discriminator_type_name=c_name(discriminator_type_name))
 
     for key in typeinfo:
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index e8ee2688e6..4ec79a6db8 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -288,20 +288,23 @@ void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error **e
                      name=c_name(name))
 
     if not discriminator:
+        tag = 'kind'
         disc_key = "type"
     else:
+        tag = discriminator
         disc_key = discriminator
     ret += mcgen('''
-        visit_type_%(disc_type)s(m, &(*obj)->kind, "%(disc_key)s", &err);
+        visit_type_%(disc_type)s(m, &(*obj)->%(c_tag)s, "%(disc_key)s", &err);
         if (err) {
             goto out_obj;
         }
         if (!visit_start_union(m, !!(*obj)->data, &err) || err) {
             goto out_obj;
         }
-        switch ((*obj)->kind) {
+        switch ((*obj)->%(c_tag)s) {
 ''',
                  disc_type = disc_type,
+                 c_tag=c_name(tag),
                  disc_key = disc_key)
 
     for key in members:
diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c
index b96195309b..b7a87ee351 100644
--- a/tests/test-qmp-input-visitor.c
+++ b/tests/test-qmp-input-visitor.c
@@ -313,7 +313,7 @@ static void test_visitor_in_union_flat(TestInputVisitorData *data,
 
     visit_type_UserDefFlatUnion(v, &tmp, NULL, &err);
     g_assert(err == NULL);
-    g_assert_cmpint(tmp->kind, ==, ENUM_ONE_VALUE1);
+    g_assert_cmpint(tmp->enum1, ==, ENUM_ONE_VALUE1);
     g_assert_cmpstr(tmp->string, ==, "str");
     /* TODO g_assert_cmpint(tmp->integer, ==, 41); */
     g_assert_cmpint(tmp->value1->boolean, ==, true);
diff --git a/tests/test-qmp-output-visitor.c b/tests/test-qmp-output-visitor.c
index 87ba350b43..338ada0253 100644
--- a/tests/test-qmp-output-visitor.c
+++ b/tests/test-qmp-output-visitor.c
@@ -437,7 +437,7 @@ static void test_visitor_out_union_flat(TestOutputVisitorData *data,
     Error *err = NULL;
 
     UserDefFlatUnion *tmp = g_malloc0(sizeof(UserDefFlatUnion));
-    tmp->kind = ENUM_ONE_VALUE1;
+    tmp->enum1 = ENUM_ONE_VALUE1;
     tmp->string = g_strdup("str");
     tmp->value1 = g_malloc0(sizeof(UserDefA));
     /* TODO when generator bug is fixed: tmp->integer = 41; */

From 1e6c1616a91cdcbe9a8387541f7689b8c11632aa Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Sun, 28 Jun 2015 20:05:53 +0200
Subject: [PATCH 08/33] qapi: Generate a nicer struct for flat unions

The struct generated for a flat union is weird: the members of its
base are at the end, except for the union tag, which is at the
beginning.

Example: qapi-schema-test.json has

    { 'struct': 'UserDefUnionBase',
      'data': { 'string': 'str', 'enum1': 'EnumOne' } }

    { 'union': 'UserDefFlatUnion',
      'base': 'UserDefUnionBase',
      'discriminator': 'enum1',
      'data': { 'value1' : 'UserDefA',
                'value2' : 'UserDefB',
                'value3' : 'UserDefB' } }

We generate:

    struct UserDefFlatUnion
    {
        EnumOne enum1;
        union {
            void *data;
            UserDefA *value1;
            UserDefB *value2;
            UserDefB *value3;
        };
        char *string;
    };

Change to put all base members at the beginning, unadulterated.  Not
only is this easier to understand, it also permits casting the flat
union to its base, if that should become useful.

We now generate:

    struct UserDefFlatUnion
    {
        /* Members inherited from UserDefUnionBase: */
        char *string;
        EnumOne enum1;
        /* Own members: */
        union { /* union tag is @enum1 */
            void *data;
            UserDefA *value1;
            UserDefB *value2;
            UserDefB *value3;
        };
    };

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 scripts/qapi-types.py | 38 ++++++++++++++++++++++----------------
 1 file changed, 22 insertions(+), 16 deletions(-)

diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index ac8dad3171..82141cdec3 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -200,13 +200,30 @@ def generate_union(expr, meta):
     ret = mcgen('''
 struct %(name)s
 {
-    %(discriminator_type_name)s %(discriminator)s;
-    union {
+''',
+                name=name)
+    if base:
+        ret += mcgen('''
+    /* Members inherited from %(c_name)s: */
+''',
+                     c_name=c_name(base))
+        base_fields = find_struct(base)['data']
+        ret += generate_struct_fields(base_fields)
+        ret += mcgen('''
+    /* Own members: */
+''')
+    else:
+        assert not discriminator
+        ret += mcgen('''
+    %(discriminator_type_name)s kind;
+''',
+                     discriminator_type_name=c_name(discriminator_type_name))
+
+    ret += mcgen('''
+    union { /* union tag is @%(c_name)s */
         void *data;
 ''',
-                name=name,
-                discriminator=c_name(discriminator or 'kind'),
-                discriminator_type_name=c_name(discriminator_type_name))
+                 c_name=c_name(discriminator or 'kind'))
 
     for key in typeinfo:
         ret += mcgen('''
@@ -217,17 +234,6 @@ struct %(name)s
 
     ret += mcgen('''
     };
-''')
-
-    if base:
-        assert discriminator
-        base_fields = find_struct(base)['data'].copy()
-        del base_fields[discriminator]
-        ret += generate_struct_fields(base_fields)
-    else:
-        assert not discriminator
-
-    ret += mcgen('''
 };
 ''')
     if meta == 'alternate':

From 8c3f8e77215bfedb7854221868f655e148506936 Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Fri, 26 Jun 2015 10:19:11 +0200
Subject: [PATCH 09/33] qapi-visit: Fix generated code when schema has forward
 refs

The visit_type_implicit_FOO() are generated on demand, right before
their first use.  Used by visit_type_STRUCT_fields() when STRUCT has
base FOO, and by visit_type_UNION() when flat UNION has member a FOO.

If the schema defines FOO after its first use as struct base or flat
union member, visit_type_implicit_FOO() calls
visit_type_implicit_FOO() before its definition, which doesn't
compile.

Rearrange qapi-schema-test.json to demonstrate the bug.

Fix by generating the necessary forward declaration.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 scripts/qapi-visit.py                   | 15 +++++++++++-
 tests/qapi-schema/qapi-schema-test.json | 32 ++++++++++++++-----------
 tests/qapi-schema/qapi-schema-test.out  | 10 ++++----
 3 files changed, 37 insertions(+), 20 deletions(-)

diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 4ec79a6db8..b3a308fb51 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -17,13 +17,23 @@ from qapi import *
 import re
 
 implicit_structs = []
+struct_fields_seen = set()
 
 def generate_visit_implicit_struct(type):
     global implicit_structs
     if type in implicit_structs:
         return ''
     implicit_structs.append(type)
-    return mcgen('''
+    ret = ''
+    if type 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_name(type))
+
+    ret += mcgen('''
 
 static void visit_type_implicit_%(c_type)s(Visitor *m, %(c_type)s **obj, Error **errp)
 {
@@ -38,8 +48,11 @@ static void visit_type_implicit_%(c_type)s(Visitor *m, %(c_type)s **obj, Error *
 }
 ''',
                  c_type=type_name(type))
+    return ret
 
 def generate_visit_struct_fields(name, members, base = None):
+    struct_fields_seen.add(name)
+
     ret = ''
 
     if base:
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index c7eaa865da..ccadb13785 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -7,13 +7,13 @@
   'data': { 'enum1': 'EnumOne', '*enum2': 'EnumOne', 'enum3': 'EnumOne', '*enum4': 'EnumOne' } }
 
 # for testing nested structs
+{ 'struct': 'UserDefOne',
+  'base': 'UserDefZero',        # intentional forward reference
+  'data': { 'string': 'str', '*enum1': 'EnumOne' } }
+
 { 'struct': 'UserDefZero',
   'data': { 'integer': 'int' } }
 
-{ 'struct': 'UserDefOne',
-  'base': 'UserDefZero',
-  'data': { 'string': 'str', '*enum1': 'EnumOne' } }
-
 { 'struct': 'UserDefTwoDictDict',
   'data': { 'userdef': 'UserDefOne', 'string': 'str' } }
 
@@ -33,29 +33,33 @@
 { 'struct': 'UserDefB',
   'data': { 'integer': 'int' } }
 
-{ 'struct': 'UserDefC',
-  'data': { 'string1': 'str', 'string2': 'str' } }
+{ 'union': 'UserDefFlatUnion',
+  'base': 'UserDefUnionBase',   # intentional forward reference
+  'discriminator': 'enum1',
+  'data': { 'value1' : 'UserDefA',
+            'value2' : 'UserDefB',
+            'value3' : 'UserDefB' } }
+# FIXME generated struct UserDefFlatUnion has members for direct base
+# UserDefOne, but lacks members for indirect base UserDefZero
 
 { 'struct': 'UserDefUnionBase',
   'data': { 'string': 'str', 'enum1': 'EnumOne' } }
 
-{ 'union': 'UserDefFlatUnion',
-  'base': 'UserDefUnionBase',
-  'discriminator': 'enum1',
-  'data': { 'value1' : 'UserDefA', 'value2' : 'UserDefB', 'value3' : 'UserDefB' } }
-# FIXME generated struct UserDefFlatUnion has members for direct base
-# UserDefOne, but lacks members for indirect base UserDefZero
-
 # this variant of UserDefFlatUnion defaults to a union that uses fields with
 # allocated types to test corner cases in the cleanup/dealloc visitor
 { 'union': 'UserDefFlatUnion2',
   'base': 'UserDefUnionBase',
   'discriminator': 'enum1',
-  'data': { 'value1' : 'UserDefC', 'value2' : 'UserDefB', 'value3' : 'UserDefA' } }
+  'data': { 'value1' : 'UserDefC', # intentional forward reference
+            'value2' : 'UserDefB',
+            'value3' : 'UserDefA' } }
 
 { 'alternate': 'UserDefAlternate',
   'data': { 'uda': 'UserDefA', 's': 'str', 'i': 'int' } }
 
+{ 'struct': 'UserDefC',
+  'data': { 'string1': 'str', 'string2': 'str' } }
+
 # for testing native lists
 { 'union': 'UserDefNativeListUnion',
   'data': { 'integer': ['int'],
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index cf0ccc4025..a4291db1df 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -1,17 +1,17 @@
 [OrderedDict([('enum', 'EnumOne'), ('data', ['value1', 'value2', 'value3'])]),
  OrderedDict([('struct', 'NestedEnumsOne'), ('data', OrderedDict([('enum1', 'EnumOne'), ('*enum2', 'EnumOne'), ('enum3', 'EnumOne'), ('*enum4', 'EnumOne')]))]),
- OrderedDict([('struct', 'UserDefZero'), ('data', OrderedDict([('integer', 'int')]))]),
  OrderedDict([('struct', 'UserDefOne'), ('base', 'UserDefZero'), ('data', OrderedDict([('string', 'str'), ('*enum1', 'EnumOne')]))]),
+ OrderedDict([('struct', 'UserDefZero'), ('data', OrderedDict([('integer', 'int')]))]),
  OrderedDict([('struct', 'UserDefTwoDictDict'), ('data', OrderedDict([('userdef', 'UserDefOne'), ('string', 'str')]))]),
  OrderedDict([('struct', 'UserDefTwoDict'), ('data', OrderedDict([('string1', 'str'), ('dict2', 'UserDefTwoDictDict'), ('*dict3', 'UserDefTwoDictDict')]))]),
  OrderedDict([('struct', 'UserDefTwo'), ('data', OrderedDict([('string0', 'str'), ('dict1', 'UserDefTwoDict')]))]),
  OrderedDict([('struct', 'UserDefA'), ('data', OrderedDict([('boolean', 'bool')]))]),
  OrderedDict([('struct', 'UserDefB'), ('data', OrderedDict([('integer', 'int')]))]),
- OrderedDict([('struct', 'UserDefC'), ('data', OrderedDict([('string1', 'str'), ('string2', 'str')]))]),
- OrderedDict([('struct', 'UserDefUnionBase'), ('data', OrderedDict([('string', 'str'), ('enum1', 'EnumOne')]))]),
  OrderedDict([('union', 'UserDefFlatUnion'), ('base', 'UserDefUnionBase'), ('discriminator', 'enum1'), ('data', OrderedDict([('value1', 'UserDefA'), ('value2', 'UserDefB'), ('value3', 'UserDefB')]))]),
+ OrderedDict([('struct', 'UserDefUnionBase'), ('data', OrderedDict([('string', 'str'), ('enum1', 'EnumOne')]))]),
  OrderedDict([('union', 'UserDefFlatUnion2'), ('base', 'UserDefUnionBase'), ('discriminator', 'enum1'), ('data', OrderedDict([('value1', 'UserDefC'), ('value2', 'UserDefB'), ('value3', 'UserDefA')]))]),
  OrderedDict([('alternate', 'UserDefAlternate'), ('data', OrderedDict([('uda', 'UserDefA'), ('s', 'str'), ('i', 'int')]))]),
+ OrderedDict([('struct', 'UserDefC'), ('data', OrderedDict([('string1', 'str'), ('string2', 'str')]))]),
  OrderedDict([('union', 'UserDefNativeListUnion'), ('data', OrderedDict([('integer', ['int']), ('s8', ['int8']), ('s16', ['int16']), ('s32', ['int32']), ('s64', ['int64']), ('u8', ['uint8']), ('u16', ['uint16']), ('u32', ['uint32']), ('u64', ['uint64']), ('number', ['number']), ('boolean', ['bool']), ('string', ['str']), ('sizes', ['size'])]))]),
  OrderedDict([('command', 'user_def_cmd'), ('data', OrderedDict())]),
  OrderedDict([('command', 'user_def_cmd1'), ('data', OrderedDict([('ud1a', 'UserDefOne')]))]),
@@ -39,15 +39,15 @@
  {'enum_name': '__org.qemu_x-Union1Kind', 'enum_values': None},
  {'enum_name': '__org.qemu_x-AltKind', 'enum_values': None}]
 [OrderedDict([('struct', 'NestedEnumsOne'), ('data', OrderedDict([('enum1', 'EnumOne'), ('*enum2', 'EnumOne'), ('enum3', 'EnumOne'), ('*enum4', 'EnumOne')]))]),
- OrderedDict([('struct', 'UserDefZero'), ('data', OrderedDict([('integer', 'int')]))]),
  OrderedDict([('struct', 'UserDefOne'), ('base', 'UserDefZero'), ('data', OrderedDict([('string', 'str'), ('*enum1', 'EnumOne')]))]),
+ OrderedDict([('struct', 'UserDefZero'), ('data', OrderedDict([('integer', 'int')]))]),
  OrderedDict([('struct', 'UserDefTwoDictDict'), ('data', OrderedDict([('userdef', 'UserDefOne'), ('string', 'str')]))]),
  OrderedDict([('struct', 'UserDefTwoDict'), ('data', OrderedDict([('string1', 'str'), ('dict2', 'UserDefTwoDictDict'), ('*dict3', 'UserDefTwoDictDict')]))]),
  OrderedDict([('struct', 'UserDefTwo'), ('data', OrderedDict([('string0', 'str'), ('dict1', 'UserDefTwoDict')]))]),
  OrderedDict([('struct', 'UserDefA'), ('data', OrderedDict([('boolean', 'bool')]))]),
  OrderedDict([('struct', 'UserDefB'), ('data', OrderedDict([('integer', 'int')]))]),
- OrderedDict([('struct', 'UserDefC'), ('data', OrderedDict([('string1', 'str'), ('string2', 'str')]))]),
  OrderedDict([('struct', 'UserDefUnionBase'), ('data', OrderedDict([('string', 'str'), ('enum1', 'EnumOne')]))]),
+ OrderedDict([('struct', 'UserDefC'), ('data', OrderedDict([('string1', 'str'), ('string2', 'str')]))]),
  OrderedDict([('struct', 'UserDefOptions'), ('data', OrderedDict([('*i64', ['int']), ('*u64', ['uint64']), ('*u16', ['uint16']), ('*i64x', 'int'), ('*u64x', 'uint64')]))]),
  OrderedDict([('struct', 'EventStructOne'), ('data', OrderedDict([('struct1', 'UserDefOne'), ('string', 'str'), ('*enum2', 'EnumOne')]))]),
  OrderedDict([('struct', '__org.qemu_x-Base'), ('data', OrderedDict([('__org.qemu_x-member1', '__org.qemu_x-Enum')]))]),

From 8c07eddc619d618965fdd7a96bfe3b5c59f42b52 Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Tue, 30 Jun 2015 09:27:04 +0200
Subject: [PATCH 10/33] qapi-visit: Replace list implicit_structs by set

Use set because that's what it is.  While there, rename to
implicit_structs_seen.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 scripts/qapi-visit.py | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index b3a308fb51..9fc040e0f6 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -16,14 +16,13 @@ from ordereddict import OrderedDict
 from qapi import *
 import re
 
-implicit_structs = []
+implicit_structs_seen = set()
 struct_fields_seen = set()
 
 def generate_visit_implicit_struct(type):
-    global implicit_structs
-    if type in implicit_structs:
+    if type in implicit_structs_seen:
         return ''
-    implicit_structs.append(type)
+    implicit_structs_seen.add(type)
     ret = ''
     if type not in struct_fields_seen:
         # Need a forward declaration

From 40b3adec13a9e022ff5a2e2b81c243fc0a026746 Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Fri, 26 Jun 2015 17:21:42 +0200
Subject: [PATCH 11/33] qapi-visit: Fix two name arguments passed to visitors

The generated code passes mangled schema names to visit_type_enum()
and union's visit_start_struct().  Fix it to pass the names
unadulterated, like we do everywhere else.

Only qapi-schema-test.json actually has names where this makes a
difference: enum __org.qemu_x-Enum, flat union __org.qemu_x-Union2,
simple union __org.qemu_x-Union1 and its implicit enum
__org.qemu_x-Union1Kind.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 scripts/qapi-visit.py | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 9fc040e0f6..73f136fc1d 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -185,12 +185,12 @@ out:
 def generate_visit_enum(name):
     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)
 {
-    visit_type_enum(m, (int *)obj, %(name)s_lookup, "%(name)s", name, errp);
+    visit_type_enum(m, (int *)obj, %(c_name)s_lookup, "%(name)s", name, errp);
 }
 ''',
-                 name=c_name(name))
+                 c_name=c_name(name), name=name)
 
 def generate_visit_alternate(name, members):
     ret = mcgen('''
@@ -278,17 +278,17 @@ def generate_visit_union(expr):
 
     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_struct(m, (void **)obj, "%(name)s", name, sizeof(%(name)s), &err);
+    visit_start_struct(m, (void **)obj, "%(name)s", name, sizeof(%(c_name)s), &err);
     if (err) {
         goto out;
     }
     if (*obj) {
 ''',
-                 name=c_name(name))
+                 c_name=c_name(name), name=name)
 
     if base:
         ret += mcgen('''

From 422e16aac4bd4476f5b40bee3049089de34ef6b6 Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Fri, 26 Jun 2015 17:52:45 +0200
Subject: [PATCH 12/33] tests/qapi-schema: Document alternate's enum lacks
 visit function

We generate a declaration, but no definition.

The QMP schema has two: Qcow2OverlapChecks and BlockdevRef.  Neither
visit_type_Qcow2OverlapChecksKind() nor visit_type_BlockdevRefKind()
is actually used.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 tests/qapi-schema/qapi-schema-test.json | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index ccadb13785..8337ba941f 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -56,6 +56,7 @@
 
 { 'alternate': 'UserDefAlternate',
   'data': { 'uda': 'UserDefA', 's': 'str', 'i': 'int' } }
+# FIXME only a declaration of visit_type_UserDefAlternateKind() generated
 
 { 'struct': 'UserDefC',
   'data': { 'string1': 'str', 'string2': 'str' } }

From 999387782f736d7ac0083f4f02e2bc4ce7a9a27b Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Fri, 26 Jun 2015 13:14:02 +0200
Subject: [PATCH 13/33] tests/qapi-schema: Document events with base don't work

When event FOO's 'data' is a struct with a base, we consider only the
struct's direct members, and ignore its base.  The generated
qapi_event_send_foo() doesn't take arguments for base members.

No such events currently exist in the QMP schema.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 tests/qapi-schema/qapi-schema-test.json | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index 8337ba941f..829dd30d7f 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -128,6 +128,9 @@
 { 'alternate': '__org.qemu_x-Alt',
   'data': { '__org.qemu_x-branch': 'str', 'b': '__org.qemu_x-Base' } }
 { 'event': '__ORG.QEMU_X-EVENT', 'data': '__org.qemu_x-Struct' }
+# FIXME generated qapi_event_send___org_qemu_x_event() has only a
+# parameter for data's member __org_qemu_x_member2, none for its base
+# __org.qemu_x-Base's member __org_qemu_x_member1
 { 'command': '__org.qemu_x-command',
   'data': { 'a': ['__org.qemu_x-Enum'], 'b': ['__org.qemu_x-Struct'],
             'c': '__org.qemu_x-Union2', 'd': '__org.qemu_x-Alt' },

From 2f52e20597ebd55ede668b2b7d162a84f419b03e Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@redhat.com>
Date: Thu, 30 Jul 2015 16:33:07 -0600
Subject: [PATCH 14/33] qapi: Document that input visitor semantics are prone
 to leaks

Most functions that can return a pointer or set an Error ** value
are decent enough to guarantee a NULL return when reporting an error.
Not so with our generated qapi visitor functions.  If the caller
is not careful to clean up partially-allocated objects on error,
then the caller suffers a memory leak.

Properly fixing it is probably complex enough to save for a later
day, so merely document it for now.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1438295587-19069-1-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/qapi-visit.py          | 4 ++++
 tests/test-qmp-input-visitor.c | 2 ++
 2 files changed, 6 insertions(+)

diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 73f136fc1d..eec5f1f4c5 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -115,6 +115,10 @@ out:
 
 
 def generate_visit_struct_body(name):
+    # FIXME: if *obj is NULL on entry, and visit_start_struct() assigns to
+    # *obj, but then visit_type_FOO_fields() fails, we should clean up *obj
+    # rather than leaving it non-NULL. As currently written, the caller must
+    # call qapi_free_FOO() to avoid a memory leak of the partial FOO.
     ret = mcgen('''
     Error *err = NULL;
 
diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c
index b7a87ee351..a5cfefae8b 100644
--- a/tests/test-qmp-input-visitor.c
+++ b/tests/test-qmp-input-visitor.c
@@ -636,6 +636,8 @@ static void test_visitor_in_errors(TestInputVisitorData *data,
 
     visit_type_TestStruct(v, &p, NULL, &err);
     g_assert(err);
+    /* FIXME - a failed parse should not leave a partially-allocated p
+     * for us to clean up; this could cause callers to leak memory. */
     g_assert(p->string == NULL);
 
     error_free(err);

From ca56a822dd538017715345cbbe1f8829e0cc2742 Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@redhat.com>
Date: Thu, 30 Jul 2015 17:07:17 -0600
Subject: [PATCH 15/33] qapi: Document shortcoming with union 'data' branch

Add a FIXME to remind us to fully audit whether removing the
'void *data' branch of each qapi union type can be done safely.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1438297637-26789-1-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/qapi-types.py | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 82141cdec3..8444f9836a 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -219,6 +219,14 @@ struct %(name)s
 ''',
                      discriminator_type_name=c_name(discriminator_type_name))
 
+    # FIXME: What purpose does data serve, besides preventing a union that
+    # has a branch named 'data'? We use it in qapi-visit.py to decide
+    # whether to bypass the switch statement if visiting the discriminator
+    # failed; but since we 0-initialize structs, and cannot tell what
+    # branch of the union is in use if the discriminator is invalid, there
+    # should not be any data leaks even without a data pointer.  Or, if
+    # 'data' is merely added to guarantee we don't have an empty union,
+    # shouldn't we enforce that at .json parse time?
     ret += mcgen('''
     union { /* union tag is @%(c_name)s */
         void *data;

From d90675fa4bc256238b3dd3a7fdd5f9029eca00b8 Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Fri, 31 Jul 2015 11:33:52 +0200
Subject: [PATCH 16/33] qapi: Document flaws in checking of names

We don't actually enforce our "other than downstream extensions [...],
all names should begin with a letter" rule.  Add a FIXME.

We should reject names that differ only in '_' vs. '.'  vs. '-',
because they're liable to clash in generated C.  Add a FIXME.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 scripts/qapi.py | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index e7c814dbc8..487998244b 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -341,6 +341,8 @@ 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):
@@ -367,6 +369,8 @@ def check_name(expr_info, source, name, allow_optional = 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. '.'
+    # vs. '-', because they're liable to clash in generated C.
     if name in all_names:
         raise QAPIExprError(info,
                             "%s '%s' is already defined"

From 80e60a19a82cf872652d1923e800fecef5cc7def Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Fri, 26 Jun 2015 13:21:10 +0200
Subject: [PATCH 17/33] tests/qapi-schema: Restore test case for flat union
 base bug

Test case added in commit 2fc0043, and messed up in commit 5223070.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 tests/qapi-schema/qapi-schema-test.json | 5 +++--
 tests/qapi-schema/qapi-schema-test.out  | 8 ++++----
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index 829dd30d7f..a9e5aab927 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -31,7 +31,7 @@
   'data': { 'boolean': 'bool' } }
 
 { 'struct': 'UserDefB',
-  'data': { 'integer': 'int' } }
+  'data': { 'intb': 'int' } }
 
 { 'union': 'UserDefFlatUnion',
   'base': 'UserDefUnionBase',   # intentional forward reference
@@ -40,9 +40,10 @@
             'value2' : 'UserDefB',
             'value3' : 'UserDefB' } }
 # FIXME generated struct UserDefFlatUnion has members for direct base
-# UserDefOne, but lacks members for indirect base UserDefZero
+# UserDefUnionBase, but lacks members for indirect base UserDefZero
 
 { 'struct': 'UserDefUnionBase',
+  'base': 'UserDefZero',
   'data': { 'string': 'str', 'enum1': 'EnumOne' } }
 
 # this variant of UserDefFlatUnion defaults to a union that uses fields with
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index a4291db1df..b0b7187226 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -6,9 +6,9 @@
  OrderedDict([('struct', 'UserDefTwoDict'), ('data', OrderedDict([('string1', 'str'), ('dict2', 'UserDefTwoDictDict'), ('*dict3', 'UserDefTwoDictDict')]))]),
  OrderedDict([('struct', 'UserDefTwo'), ('data', OrderedDict([('string0', 'str'), ('dict1', 'UserDefTwoDict')]))]),
  OrderedDict([('struct', 'UserDefA'), ('data', OrderedDict([('boolean', 'bool')]))]),
- OrderedDict([('struct', 'UserDefB'), ('data', OrderedDict([('integer', 'int')]))]),
+ OrderedDict([('struct', 'UserDefB'), ('data', OrderedDict([('intb', 'int')]))]),
  OrderedDict([('union', 'UserDefFlatUnion'), ('base', 'UserDefUnionBase'), ('discriminator', 'enum1'), ('data', OrderedDict([('value1', 'UserDefA'), ('value2', 'UserDefB'), ('value3', 'UserDefB')]))]),
- OrderedDict([('struct', 'UserDefUnionBase'), ('data', OrderedDict([('string', 'str'), ('enum1', 'EnumOne')]))]),
+ OrderedDict([('struct', 'UserDefUnionBase'), ('base', 'UserDefZero'), ('data', OrderedDict([('string', 'str'), ('enum1', 'EnumOne')]))]),
  OrderedDict([('union', 'UserDefFlatUnion2'), ('base', 'UserDefUnionBase'), ('discriminator', 'enum1'), ('data', OrderedDict([('value1', 'UserDefC'), ('value2', 'UserDefB'), ('value3', 'UserDefA')]))]),
  OrderedDict([('alternate', 'UserDefAlternate'), ('data', OrderedDict([('uda', 'UserDefA'), ('s', 'str'), ('i', 'int')]))]),
  OrderedDict([('struct', 'UserDefC'), ('data', OrderedDict([('string1', 'str'), ('string2', 'str')]))]),
@@ -45,8 +45,8 @@
  OrderedDict([('struct', 'UserDefTwoDict'), ('data', OrderedDict([('string1', 'str'), ('dict2', 'UserDefTwoDictDict'), ('*dict3', 'UserDefTwoDictDict')]))]),
  OrderedDict([('struct', 'UserDefTwo'), ('data', OrderedDict([('string0', 'str'), ('dict1', 'UserDefTwoDict')]))]),
  OrderedDict([('struct', 'UserDefA'), ('data', OrderedDict([('boolean', 'bool')]))]),
- OrderedDict([('struct', 'UserDefB'), ('data', OrderedDict([('integer', 'int')]))]),
- OrderedDict([('struct', 'UserDefUnionBase'), ('data', OrderedDict([('string', 'str'), ('enum1', 'EnumOne')]))]),
+ OrderedDict([('struct', 'UserDefB'), ('data', OrderedDict([('intb', 'int')]))]),
+ OrderedDict([('struct', 'UserDefUnionBase'), ('base', 'UserDefZero'), ('data', OrderedDict([('string', 'str'), ('enum1', 'EnumOne')]))]),
  OrderedDict([('struct', 'UserDefC'), ('data', OrderedDict([('string1', 'str'), ('string2', 'str')]))]),
  OrderedDict([('struct', 'UserDefOptions'), ('data', OrderedDict([('*i64', ['int']), ('*u64', ['uint64']), ('*u16', ['uint16']), ('*i64x', 'int'), ('*u64x', 'uint64')]))]),
  OrderedDict([('struct', 'EventStructOne'), ('data', OrderedDict([('struct1', 'UserDefOne'), ('string', 'str'), ('*enum2', 'EnumOne')]))]),

From 6af9a8fc8ec83f823c079211bc7a2414b1d4e5fe Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Fri, 31 Jul 2015 13:30:50 +0200
Subject: [PATCH 18/33] tests/qapi-schema: Rename tests from data- to args-

Since every schema entity has 'data', the data- prefix conveys no
information.  These tests actually exercise commands.  Only commands
have arguments, so change the prefix to to args-.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 tests/Makefile                                              | 6 +++---
 .../{data-array-empty.err => args-array-empty.err}          | 2 +-
 .../{data-array-empty.exit => args-array-empty.exit}        | 0
 .../{data-array-empty.json => args-array-empty.json}        | 0
 .../{data-array-empty.out => args-array-empty.out}          | 0
 .../{data-array-unknown.err => args-array-unknown.err}      | 2 +-
 .../{data-array-unknown.exit => args-array-unknown.exit}    | 0
 .../{data-array-unknown.json => args-array-unknown.json}    | 0
 .../{data-array-unknown.out => args-array-unknown.out}      | 0
 tests/qapi-schema/args-int.err                              | 1 +
 tests/qapi-schema/{data-int.exit => args-int.exit}          | 0
 tests/qapi-schema/{data-int.json => args-int.json}          | 0
 tests/qapi-schema/{data-int.out => args-int.out}            | 0
 ...{data-member-array-bad.err => args-member-array-bad.err} | 2 +-
 ...ata-member-array-bad.exit => args-member-array-bad.exit} | 0
 ...ata-member-array-bad.json => args-member-array-bad.json} | 0
 ...{data-member-array-bad.out => args-member-array-bad.out} | 0
 .../{data-member-array.err => args-member-array.err}        | 0
 .../{data-member-array.exit => args-member-array.exit}      | 0
 .../{data-member-array.json => args-member-array.json}      | 0
 .../{data-member-array.out => args-member-array.out}        | 0
 tests/qapi-schema/args-member-unknown.err                   | 1 +
 .../{data-member-unknown.exit => args-member-unknown.exit}  | 0
 .../{data-member-unknown.json => args-member-unknown.json}  | 0
 .../{data-member-unknown.out => args-member-unknown.out}    | 0
 tests/qapi-schema/args-unknown.err                          | 1 +
 tests/qapi-schema/{data-unknown.exit => args-unknown.exit}  | 0
 tests/qapi-schema/{data-unknown.json => args-unknown.json}  | 0
 tests/qapi-schema/{data-unknown.out => args-unknown.out}    | 0
 tests/qapi-schema/data-int.err                              | 1 -
 tests/qapi-schema/data-member-unknown.err                   | 1 -
 tests/qapi-schema/data-unknown.err                          | 1 -
 32 files changed, 9 insertions(+), 9 deletions(-)
 rename tests/qapi-schema/{data-array-empty.err => args-array-empty.err} (50%)
 rename tests/qapi-schema/{data-array-empty.exit => args-array-empty.exit} (100%)
 rename tests/qapi-schema/{data-array-empty.json => args-array-empty.json} (100%)
 rename tests/qapi-schema/{data-array-empty.out => args-array-empty.out} (100%)
 rename tests/qapi-schema/{data-array-unknown.err => args-array-unknown.err} (50%)
 rename tests/qapi-schema/{data-array-unknown.exit => args-array-unknown.exit} (100%)
 rename tests/qapi-schema/{data-array-unknown.json => args-array-unknown.json} (100%)
 rename tests/qapi-schema/{data-array-unknown.out => args-array-unknown.out} (100%)
 create mode 100644 tests/qapi-schema/args-int.err
 rename tests/qapi-schema/{data-int.exit => args-int.exit} (100%)
 rename tests/qapi-schema/{data-int.json => args-int.json} (100%)
 rename tests/qapi-schema/{data-int.out => args-int.out} (100%)
 rename tests/qapi-schema/{data-member-array-bad.err => args-member-array-bad.err} (52%)
 rename tests/qapi-schema/{data-member-array-bad.exit => args-member-array-bad.exit} (100%)
 rename tests/qapi-schema/{data-member-array-bad.json => args-member-array-bad.json} (100%)
 rename tests/qapi-schema/{data-member-array-bad.out => args-member-array-bad.out} (100%)
 rename tests/qapi-schema/{data-member-array.err => args-member-array.err} (100%)
 rename tests/qapi-schema/{data-member-array.exit => args-member-array.exit} (100%)
 rename tests/qapi-schema/{data-member-array.json => args-member-array.json} (100%)
 rename tests/qapi-schema/{data-member-array.out => args-member-array.out} (100%)
 create mode 100644 tests/qapi-schema/args-member-unknown.err
 rename tests/qapi-schema/{data-member-unknown.exit => args-member-unknown.exit} (100%)
 rename tests/qapi-schema/{data-member-unknown.json => args-member-unknown.json} (100%)
 rename tests/qapi-schema/{data-member-unknown.out => args-member-unknown.out} (100%)
 create mode 100644 tests/qapi-schema/args-unknown.err
 rename tests/qapi-schema/{data-unknown.exit => args-unknown.exit} (100%)
 rename tests/qapi-schema/{data-unknown.json => args-unknown.json} (100%)
 rename tests/qapi-schema/{data-unknown.out => args-unknown.out} (100%)
 delete mode 100644 tests/qapi-schema/data-int.err
 delete mode 100644 tests/qapi-schema/data-member-unknown.err
 delete mode 100644 tests/qapi-schema/data-unknown.err

diff --git a/tests/Makefile b/tests/Makefile
index 52711237ca..0d560c51aa 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -229,9 +229,9 @@ check-qapi-schema-y := $(addprefix tests/qapi-schema/, \
 	redefined-type.json redefined-command.json redefined-builtin.json \
 	redefined-event.json command-int.json bad-data.json event-max.json \
 	type-bypass.json type-bypass-no-gen.json type-bypass-bad-gen.json \
-	data-array-empty.json data-array-unknown.json data-int.json \
-	data-unknown.json data-member-unknown.json data-member-array.json \
-	data-member-array-bad.json returns-array-bad.json returns-int.json \
+	args-array-empty.json args-array-unknown.json args-int.json \
+	args-unknown.json args-member-unknown.json args-member-array.json \
+	args-member-array-bad.json returns-array-bad.json returns-int.json \
 	returns-unknown.json returns-alternate.json returns-whitelist.json \
 	missing-colon.json missing-comma-list.json missing-comma-object.json \
 	nested-struct-data.json nested-struct-returns.json non-objects.json \
diff --git a/tests/qapi-schema/data-array-empty.err b/tests/qapi-schema/args-array-empty.err
similarity index 50%
rename from tests/qapi-schema/data-array-empty.err
rename to tests/qapi-schema/args-array-empty.err
index f713f14893..cb7ed33b3f 100644
--- a/tests/qapi-schema/data-array-empty.err
+++ b/tests/qapi-schema/args-array-empty.err
@@ -1 +1 @@
-tests/qapi-schema/data-array-empty.json:2: Member 'empty' of 'data' for command 'oops': array type must contain single type name
+tests/qapi-schema/args-array-empty.json:2: Member 'empty' of 'data' for command 'oops': array type must contain single type name
diff --git a/tests/qapi-schema/data-array-empty.exit b/tests/qapi-schema/args-array-empty.exit
similarity index 100%
rename from tests/qapi-schema/data-array-empty.exit
rename to tests/qapi-schema/args-array-empty.exit
diff --git a/tests/qapi-schema/data-array-empty.json b/tests/qapi-schema/args-array-empty.json
similarity index 100%
rename from tests/qapi-schema/data-array-empty.json
rename to tests/qapi-schema/args-array-empty.json
diff --git a/tests/qapi-schema/data-array-empty.out b/tests/qapi-schema/args-array-empty.out
similarity index 100%
rename from tests/qapi-schema/data-array-empty.out
rename to tests/qapi-schema/args-array-empty.out
diff --git a/tests/qapi-schema/data-array-unknown.err b/tests/qapi-schema/args-array-unknown.err
similarity index 50%
rename from tests/qapi-schema/data-array-unknown.err
rename to tests/qapi-schema/args-array-unknown.err
index 8b731bbcc8..7834d11bc1 100644
--- a/tests/qapi-schema/data-array-unknown.err
+++ b/tests/qapi-schema/args-array-unknown.err
@@ -1 +1 @@
-tests/qapi-schema/data-array-unknown.json:2: Member 'array' of 'data' for command 'oops' uses unknown type 'array of NoSuchType'
+tests/qapi-schema/args-array-unknown.json:2: Member 'array' of 'data' for command 'oops' uses unknown type 'array of NoSuchType'
diff --git a/tests/qapi-schema/data-array-unknown.exit b/tests/qapi-schema/args-array-unknown.exit
similarity index 100%
rename from tests/qapi-schema/data-array-unknown.exit
rename to tests/qapi-schema/args-array-unknown.exit
diff --git a/tests/qapi-schema/data-array-unknown.json b/tests/qapi-schema/args-array-unknown.json
similarity index 100%
rename from tests/qapi-schema/data-array-unknown.json
rename to tests/qapi-schema/args-array-unknown.json
diff --git a/tests/qapi-schema/data-array-unknown.out b/tests/qapi-schema/args-array-unknown.out
similarity index 100%
rename from tests/qapi-schema/data-array-unknown.out
rename to tests/qapi-schema/args-array-unknown.out
diff --git a/tests/qapi-schema/args-int.err b/tests/qapi-schema/args-int.err
new file mode 100644
index 0000000000..dc1d2504ff
--- /dev/null
+++ b/tests/qapi-schema/args-int.err
@@ -0,0 +1 @@
+tests/qapi-schema/args-int.json:2: 'data' for command 'oops' cannot use built-in type 'int'
diff --git a/tests/qapi-schema/data-int.exit b/tests/qapi-schema/args-int.exit
similarity index 100%
rename from tests/qapi-schema/data-int.exit
rename to tests/qapi-schema/args-int.exit
diff --git a/tests/qapi-schema/data-int.json b/tests/qapi-schema/args-int.json
similarity index 100%
rename from tests/qapi-schema/data-int.json
rename to tests/qapi-schema/args-int.json
diff --git a/tests/qapi-schema/data-int.out b/tests/qapi-schema/args-int.out
similarity index 100%
rename from tests/qapi-schema/data-int.out
rename to tests/qapi-schema/args-int.out
diff --git a/tests/qapi-schema/data-member-array-bad.err b/tests/qapi-schema/args-member-array-bad.err
similarity index 52%
rename from tests/qapi-schema/data-member-array-bad.err
rename to tests/qapi-schema/args-member-array-bad.err
index 2c072d5986..881b4d954f 100644
--- a/tests/qapi-schema/data-member-array-bad.err
+++ b/tests/qapi-schema/args-member-array-bad.err
@@ -1 +1 @@
-tests/qapi-schema/data-member-array-bad.json:2: Member 'member' of 'data' for command 'oops': array type must contain single type name
+tests/qapi-schema/args-member-array-bad.json:2: Member 'member' of 'data' for command 'oops': array type must contain single type name
diff --git a/tests/qapi-schema/data-member-array-bad.exit b/tests/qapi-schema/args-member-array-bad.exit
similarity index 100%
rename from tests/qapi-schema/data-member-array-bad.exit
rename to tests/qapi-schema/args-member-array-bad.exit
diff --git a/tests/qapi-schema/data-member-array-bad.json b/tests/qapi-schema/args-member-array-bad.json
similarity index 100%
rename from tests/qapi-schema/data-member-array-bad.json
rename to tests/qapi-schema/args-member-array-bad.json
diff --git a/tests/qapi-schema/data-member-array-bad.out b/tests/qapi-schema/args-member-array-bad.out
similarity index 100%
rename from tests/qapi-schema/data-member-array-bad.out
rename to tests/qapi-schema/args-member-array-bad.out
diff --git a/tests/qapi-schema/data-member-array.err b/tests/qapi-schema/args-member-array.err
similarity index 100%
rename from tests/qapi-schema/data-member-array.err
rename to tests/qapi-schema/args-member-array.err
diff --git a/tests/qapi-schema/data-member-array.exit b/tests/qapi-schema/args-member-array.exit
similarity index 100%
rename from tests/qapi-schema/data-member-array.exit
rename to tests/qapi-schema/args-member-array.exit
diff --git a/tests/qapi-schema/data-member-array.json b/tests/qapi-schema/args-member-array.json
similarity index 100%
rename from tests/qapi-schema/data-member-array.json
rename to tests/qapi-schema/args-member-array.json
diff --git a/tests/qapi-schema/data-member-array.out b/tests/qapi-schema/args-member-array.out
similarity index 100%
rename from tests/qapi-schema/data-member-array.out
rename to tests/qapi-schema/args-member-array.out
diff --git a/tests/qapi-schema/args-member-unknown.err b/tests/qapi-schema/args-member-unknown.err
new file mode 100644
index 0000000000..f6f82828ce
--- /dev/null
+++ b/tests/qapi-schema/args-member-unknown.err
@@ -0,0 +1 @@
+tests/qapi-schema/args-member-unknown.json:2: Member 'member' of 'data' for command 'oops' uses unknown type 'NoSuchType'
diff --git a/tests/qapi-schema/data-member-unknown.exit b/tests/qapi-schema/args-member-unknown.exit
similarity index 100%
rename from tests/qapi-schema/data-member-unknown.exit
rename to tests/qapi-schema/args-member-unknown.exit
diff --git a/tests/qapi-schema/data-member-unknown.json b/tests/qapi-schema/args-member-unknown.json
similarity index 100%
rename from tests/qapi-schema/data-member-unknown.json
rename to tests/qapi-schema/args-member-unknown.json
diff --git a/tests/qapi-schema/data-member-unknown.out b/tests/qapi-schema/args-member-unknown.out
similarity index 100%
rename from tests/qapi-schema/data-member-unknown.out
rename to tests/qapi-schema/args-member-unknown.out
diff --git a/tests/qapi-schema/args-unknown.err b/tests/qapi-schema/args-unknown.err
new file mode 100644
index 0000000000..4d91ec869f
--- /dev/null
+++ b/tests/qapi-schema/args-unknown.err
@@ -0,0 +1 @@
+tests/qapi-schema/args-unknown.json:2: 'data' for command 'oops' uses unknown type 'NoSuchType'
diff --git a/tests/qapi-schema/data-unknown.exit b/tests/qapi-schema/args-unknown.exit
similarity index 100%
rename from tests/qapi-schema/data-unknown.exit
rename to tests/qapi-schema/args-unknown.exit
diff --git a/tests/qapi-schema/data-unknown.json b/tests/qapi-schema/args-unknown.json
similarity index 100%
rename from tests/qapi-schema/data-unknown.json
rename to tests/qapi-schema/args-unknown.json
diff --git a/tests/qapi-schema/data-unknown.out b/tests/qapi-schema/args-unknown.out
similarity index 100%
rename from tests/qapi-schema/data-unknown.out
rename to tests/qapi-schema/args-unknown.out
diff --git a/tests/qapi-schema/data-int.err b/tests/qapi-schema/data-int.err
deleted file mode 100644
index 1a9b077c06..0000000000
--- a/tests/qapi-schema/data-int.err
+++ /dev/null
@@ -1 +0,0 @@
-tests/qapi-schema/data-int.json:2: 'data' for command 'oops' cannot use built-in type 'int'
diff --git a/tests/qapi-schema/data-member-unknown.err b/tests/qapi-schema/data-member-unknown.err
deleted file mode 100644
index ab905db802..0000000000
--- a/tests/qapi-schema/data-member-unknown.err
+++ /dev/null
@@ -1 +0,0 @@
-tests/qapi-schema/data-member-unknown.json:2: Member 'member' of 'data' for command 'oops' uses unknown type 'NoSuchType'
diff --git a/tests/qapi-schema/data-unknown.err b/tests/qapi-schema/data-unknown.err
deleted file mode 100644
index 5b07277a95..0000000000
--- a/tests/qapi-schema/data-unknown.err
+++ /dev/null
@@ -1 +0,0 @@
-tests/qapi-schema/data-unknown.json:2: 'data' for command 'oops' uses unknown type 'NoSuchType'

From d9658d58e33128df32093b7a84bed76b527fb884 Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Wed, 1 Jul 2015 09:54:11 +0200
Subject: [PATCH 19/33] qapi-tests: New tests for union, alternate command
 arguments

A command's 'data' must be a struct type, given either as a
dictionary, or as struct type name.

Existing test case data-int.json covers simple type 'int'.  Add test
cases for type names referring to union and alternate types.

The latter is caught (good), but the former is not (bug).

Events have the same problem, but since they get checked by the same
code, we don't bother to duplicate the tests.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 tests/Makefile                        | 3 ++-
 tests/qapi-schema/args-alternate.err  | 1 +
 tests/qapi-schema/args-alternate.exit | 1 +
 tests/qapi-schema/args-alternate.json | 3 +++
 tests/qapi-schema/args-alternate.out  | 0
 tests/qapi-schema/args-union.err      | 0
 tests/qapi-schema/args-union.exit     | 1 +
 tests/qapi-schema/args-union.json     | 4 ++++
 tests/qapi-schema/args-union.out      | 4 ++++
 9 files changed, 16 insertions(+), 1 deletion(-)
 create mode 100644 tests/qapi-schema/args-alternate.err
 create mode 100644 tests/qapi-schema/args-alternate.exit
 create mode 100644 tests/qapi-schema/args-alternate.json
 create mode 100644 tests/qapi-schema/args-alternate.out
 create mode 100644 tests/qapi-schema/args-union.err
 create mode 100644 tests/qapi-schema/args-union.exit
 create mode 100644 tests/qapi-schema/args-union.json
 create mode 100644 tests/qapi-schema/args-union.out

diff --git a/tests/Makefile b/tests/Makefile
index 0d560c51aa..7315258789 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -231,7 +231,8 @@ check-qapi-schema-y := $(addprefix tests/qapi-schema/, \
 	type-bypass.json type-bypass-no-gen.json type-bypass-bad-gen.json \
 	args-array-empty.json args-array-unknown.json args-int.json \
 	args-unknown.json args-member-unknown.json args-member-array.json \
-	args-member-array-bad.json returns-array-bad.json returns-int.json \
+	args-member-array-bad.json args-alternate.json args-union.json \
+	returns-array-bad.json returns-int.json \
 	returns-unknown.json returns-alternate.json returns-whitelist.json \
 	missing-colon.json missing-comma-list.json missing-comma-object.json \
 	nested-struct-data.json nested-struct-returns.json non-objects.json \
diff --git a/tests/qapi-schema/args-alternate.err b/tests/qapi-schema/args-alternate.err
new file mode 100644
index 0000000000..3086eae56b
--- /dev/null
+++ b/tests/qapi-schema/args-alternate.err
@@ -0,0 +1 @@
+tests/qapi-schema/args-alternate.json:3: 'data' for command 'oops' cannot use alternate type 'Alt'
diff --git a/tests/qapi-schema/args-alternate.exit b/tests/qapi-schema/args-alternate.exit
new file mode 100644
index 0000000000..d00491fd7e
--- /dev/null
+++ b/tests/qapi-schema/args-alternate.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/args-alternate.json b/tests/qapi-schema/args-alternate.json
new file mode 100644
index 0000000000..69e94d4819
--- /dev/null
+++ b/tests/qapi-schema/args-alternate.json
@@ -0,0 +1,3 @@
+# we do not allow alternate arguments
+{ 'alternate': 'Alt', 'data': { 'case1': 'int', 'case2': 'str' } }
+{ 'command': 'oops', 'data': 'Alt' }
diff --git a/tests/qapi-schema/args-alternate.out b/tests/qapi-schema/args-alternate.out
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/qapi-schema/args-union.err b/tests/qapi-schema/args-union.err
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/qapi-schema/args-union.exit b/tests/qapi-schema/args-union.exit
new file mode 100644
index 0000000000..573541ac97
--- /dev/null
+++ b/tests/qapi-schema/args-union.exit
@@ -0,0 +1 @@
+0
diff --git a/tests/qapi-schema/args-union.json b/tests/qapi-schema/args-union.json
new file mode 100644
index 0000000000..db97ef643b
--- /dev/null
+++ b/tests/qapi-schema/args-union.json
@@ -0,0 +1,4 @@
+# FIXME we should reject union arguments
+# TODO should we support this?
+{ 'union': 'Uni', 'data': { 'case1': 'int', 'case2': 'str' } }
+{ 'command': 'oops', 'data': 'Uni' }
diff --git a/tests/qapi-schema/args-union.out b/tests/qapi-schema/args-union.out
new file mode 100644
index 0000000000..907080cce6
--- /dev/null
+++ b/tests/qapi-schema/args-union.out
@@ -0,0 +1,4 @@
+[OrderedDict([('union', 'Uni'), ('data', OrderedDict([('case1', 'int'), ('case2', 'str')]))]),
+ OrderedDict([('command', 'oops'), ('data', 'Uni')])]
+[{'enum_name': 'UniKind', 'enum_values': None}]
+[]

From 315932b5edb86597adafbd1faa2d29c46499d8c3 Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Wed, 1 Jul 2015 10:12:24 +0200
Subject: [PATCH 20/33] qapi: Fix to reject union command and event arguments

A command's or event's 'data' must be a struct type, given either as a
dictionary, or as struct type name.

Commit dd883c6 tightened the checking there, but not enough: we still
accept 'union'.  Fix to reject it.

We may want to support union types there, but we'll have to extend
qapi-commands.py and qapi-events.py for it.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 docs/qapi-code-gen.txt            | 7 +++----
 scripts/qapi.py                   | 4 ++--
 tests/qapi-schema/args-union.err  | 1 +
 tests/qapi-schema/args-union.exit | 2 +-
 tests/qapi-schema/args-union.json | 2 +-
 tests/qapi-schema/args-union.out  | 4 ----
 6 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index c2ac21c889..c19c15732d 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -405,10 +405,9 @@ Client JSON Protocol command exchange.
 The 'data' argument maps to the "arguments" dictionary passed in as
 part of a Client JSON Protocol command.  The 'data' member is optional
 and defaults to {} (an empty dictionary).  If present, it must be the
-string name of a complex type, a one-element array containing the name
-of a complex type, or a dictionary that declares an anonymous type
-with the same semantics as a 'struct' expression, with one exception
-noted below when 'gen' is used.
+string name of a complex type, or a dictionary that declares an
+anonymous type with the same semantics as a 'struct' expression, with
+one exception noted below when 'gen' is used.
 
 The 'returns' member describes what will appear in the "return" field
 of a Client JSON Protocol reply on successful completion of a command.
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 487998244b..bbeae4d322 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -499,7 +499,7 @@ def check_command(expr, expr_info):
 
     check_type(expr_info, "'data' for command '%s'" % name,
                expr.get('data'), allow_dict=True, allow_optional=True,
-               allow_metas=['union', 'struct'], allow_star=allow_star)
+               allow_metas=['struct'], allow_star=allow_star)
     returns_meta = ['union', 'struct']
     if name in returns_whitelist:
         returns_meta += ['built-in', 'alternate', 'enum']
@@ -517,7 +517,7 @@ def check_event(expr, expr_info):
     events.append(name)
     check_type(expr_info, "'data' for event '%s'" % name,
                expr.get('data'), allow_dict=True, allow_optional=True,
-               allow_metas=['union', 'struct'])
+               allow_metas=['struct'])
 
 def check_union(expr, expr_info):
     name = expr['union']
diff --git a/tests/qapi-schema/args-union.err b/tests/qapi-schema/args-union.err
index e69de29bb2..1d693d74da 100644
--- a/tests/qapi-schema/args-union.err
+++ b/tests/qapi-schema/args-union.err
@@ -0,0 +1 @@
+tests/qapi-schema/args-union.json:4: 'data' for command 'oops' cannot use union type 'Uni'
diff --git a/tests/qapi-schema/args-union.exit b/tests/qapi-schema/args-union.exit
index 573541ac97..d00491fd7e 100644
--- a/tests/qapi-schema/args-union.exit
+++ b/tests/qapi-schema/args-union.exit
@@ -1 +1 @@
-0
+1
diff --git a/tests/qapi-schema/args-union.json b/tests/qapi-schema/args-union.json
index db97ef643b..7bdcbb7f08 100644
--- a/tests/qapi-schema/args-union.json
+++ b/tests/qapi-schema/args-union.json
@@ -1,4 +1,4 @@
-# FIXME we should reject union arguments
+# we do not allow union arguments
 # TODO should we support this?
 { 'union': 'Uni', 'data': { 'case1': 'int', 'case2': 'str' } }
 { 'command': 'oops', 'data': 'Uni' }
diff --git a/tests/qapi-schema/args-union.out b/tests/qapi-schema/args-union.out
index 907080cce6..e69de29bb2 100644
--- a/tests/qapi-schema/args-union.out
+++ b/tests/qapi-schema/args-union.out
@@ -1,4 +0,0 @@
-[OrderedDict([('union', 'Uni'), ('data', OrderedDict([('case1', 'int'), ('case2', 'str')]))]),
- OrderedDict([('command', 'oops'), ('data', 'Uni')])]
-[{'enum_name': 'UniKind', 'enum_values': None}]
-[]

From 9b090d42aea9a0abbf39a1d75561a186057b5fe6 Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Fri, 31 Jul 2015 17:59:38 +0200
Subject: [PATCH 21/33] qapi: Command returning anonymous type doesn't work,
 outlaw

Reproducer: with

    { 'command': 'user_def_cmd4', 'returns': { 'a': 'int' } }

added to qapi-schema-test.json, qapi-commands.py dies when it tries to
generate the command handler function

    Traceback (most recent call last):
      File "/work/armbru/qemu/scripts/qapi-commands.py", line 359, in <module>
        ret = generate_command_decl(cmd['command'], arglist, ret_type) + "\n"
      File "/work/armbru/qemu/scripts/qapi-commands.py", line 29, in generate_command_decl
        ret_type=c_type(ret_type), name=c_name(name),
      File "/work/armbru/qemu/scripts/qapi.py", line 927, in c_type
        assert isinstance(value, str) and value != ""
    AssertionError

because the return type doesn't exist.

Simply outlaw this usage, and drop or dumb down test cases accordingly.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 docs/qapi-code-gen.txt                          | 17 ++++++++---------
 scripts/qapi.py                                 |  2 +-
 tests/Makefile                                  |  4 ++--
 tests/qapi-schema/command-int.json              |  3 +--
 tests/qapi-schema/nested-struct-data.json       |  3 +--
 tests/qapi-schema/nested-struct-returns.err     |  1 -
 tests/qapi-schema/nested-struct-returns.json    |  3 ---
 tests/qapi-schema/returns-dict.err              |  1 +
 ...ed-struct-returns.exit => returns-dict.exit} |  0
 tests/qapi-schema/returns-dict.json             |  2 ++
 ...sted-struct-returns.out => returns-dict.out} |  0
 11 files changed, 16 insertions(+), 20 deletions(-)
 delete mode 100644 tests/qapi-schema/nested-struct-returns.err
 delete mode 100644 tests/qapi-schema/nested-struct-returns.json
 create mode 100644 tests/qapi-schema/returns-dict.err
 rename tests/qapi-schema/{nested-struct-returns.exit => returns-dict.exit} (100%)
 create mode 100644 tests/qapi-schema/returns-dict.json
 rename tests/qapi-schema/{nested-struct-returns.out => returns-dict.out} (100%)

diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index c19c15732d..a253e27d5f 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -394,7 +394,7 @@ following example objects:
 === Commands ===
 
 Usage: { 'command': STRING, '*data': COMPLEX-TYPE-NAME-OR-DICT,
-         '*returns': TYPE-NAME-OR-DICT,
+         '*returns': TYPE-NAME,
          '*gen': false, '*success-response': false }
 
 Commands are defined by using a dictionary containing several members,
@@ -415,14 +415,13 @@ The member is optional from the command declaration; if absent, the
 "return" field will be an empty dictionary.  If 'returns' is present,
 it must be the string name of a complex or built-in type, a
 one-element array containing the name of a complex or built-in type,
-or a dictionary that declares an anonymous type with the same
-semantics as a 'struct' expression, with one exception noted below
-when 'gen' is used.  Although it is permitted to have the 'returns'
-member name a built-in type or an array of built-in types, any command
-that does this cannot be extended to return additional information in
-the future; thus, new commands should strongly consider returning a
-dictionary-based type or an array of dictionaries, even if the
-dictionary only contains one field at the present.
+with one exception noted below when 'gen' is used.  Although it is
+permitted to have the 'returns' member name a built-in type or an
+array of built-in types, any command that does this cannot be extended
+to return additional information in the future; thus, new commands
+should strongly consider returning a dictionary-based type or an array
+of dictionaries, even if the dictionary only contains one field at the
+present.
 
 All commands in Client JSON Protocol use a dictionary to report
 failure, with no way to specify that in QAPI.  Where the error return
diff --git a/scripts/qapi.py b/scripts/qapi.py
index bbeae4d322..23c32fe3dd 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -504,7 +504,7 @@ def check_command(expr, expr_info):
     if name in returns_whitelist:
         returns_meta += ['built-in', 'alternate', 'enum']
     check_type(expr_info, "'returns' for command '%s'" % name,
-               expr.get('returns'), allow_array=True, allow_dict=True,
+               expr.get('returns'), allow_array=True,
                allow_optional=True, allow_metas=returns_meta,
                allow_star=allow_star)
 
diff --git a/tests/Makefile b/tests/Makefile
index 7315258789..b8d445e955 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -232,10 +232,10 @@ check-qapi-schema-y := $(addprefix tests/qapi-schema/, \
 	args-array-empty.json args-array-unknown.json args-int.json \
 	args-unknown.json args-member-unknown.json args-member-array.json \
 	args-member-array-bad.json args-alternate.json args-union.json \
-	returns-array-bad.json returns-int.json \
+	returns-array-bad.json returns-int.json returns-dict.json \
 	returns-unknown.json returns-alternate.json returns-whitelist.json \
 	missing-colon.json missing-comma-list.json missing-comma-object.json \
-	nested-struct-data.json nested-struct-returns.json non-objects.json \
+	nested-struct-data.json non-objects.json \
 	qapi-schema-test.json quoted-structural-chars.json \
 	trailing-comma-list.json trailing-comma-object.json \
 	unclosed-list.json unclosed-object.json unclosed-string.json \
diff --git a/tests/qapi-schema/command-int.json b/tests/qapi-schema/command-int.json
index c90d408abe..9a62554fc6 100644
--- a/tests/qapi-schema/command-int.json
+++ b/tests/qapi-schema/command-int.json
@@ -1,3 +1,2 @@
 # we reject collisions between commands and types
-{ 'command': 'int', 'data': { 'character': 'str' },
-  'returns': { 'value': 'int' } }
+{ 'command': 'int', 'data': { 'character': 'str' } }
diff --git a/tests/qapi-schema/nested-struct-data.json b/tests/qapi-schema/nested-struct-data.json
index 3d52d2b398..efbe773ded 100644
--- a/tests/qapi-schema/nested-struct-data.json
+++ b/tests/qapi-schema/nested-struct-data.json
@@ -1,4 +1,3 @@
 # inline subtypes collide with our desired future use of defaults
 { 'command': 'foo',
-  'data': { 'a' : { 'string' : 'str', 'integer': 'int' }, 'b' : 'str' },
-  'returns': {} }
+  'data': { 'a' : { 'string' : 'str', 'integer': 'int' }, 'b' : 'str' } }
diff --git a/tests/qapi-schema/nested-struct-returns.err b/tests/qapi-schema/nested-struct-returns.err
deleted file mode 100644
index 5238d075b7..0000000000
--- a/tests/qapi-schema/nested-struct-returns.err
+++ /dev/null
@@ -1 +0,0 @@
-tests/qapi-schema/nested-struct-returns.json:2: Member 'a' of 'returns' for command 'foo' should be a type name
diff --git a/tests/qapi-schema/nested-struct-returns.json b/tests/qapi-schema/nested-struct-returns.json
deleted file mode 100644
index d2cd047f0d..0000000000
--- a/tests/qapi-schema/nested-struct-returns.json
+++ /dev/null
@@ -1,3 +0,0 @@
-# inline subtypes collide with our desired future use of defaults
-{ 'command': 'foo',
-  'returns': { 'a' : { 'string' : 'str', 'integer': 'int' }, 'b' : 'str' } }
diff --git a/tests/qapi-schema/returns-dict.err b/tests/qapi-schema/returns-dict.err
new file mode 100644
index 0000000000..eb2d0c4661
--- /dev/null
+++ b/tests/qapi-schema/returns-dict.err
@@ -0,0 +1 @@
+tests/qapi-schema/returns-dict.json:2: 'returns' for command 'oops' should be a type name
diff --git a/tests/qapi-schema/nested-struct-returns.exit b/tests/qapi-schema/returns-dict.exit
similarity index 100%
rename from tests/qapi-schema/nested-struct-returns.exit
rename to tests/qapi-schema/returns-dict.exit
diff --git a/tests/qapi-schema/returns-dict.json b/tests/qapi-schema/returns-dict.json
new file mode 100644
index 0000000000..1cfef3ede7
--- /dev/null
+++ b/tests/qapi-schema/returns-dict.json
@@ -0,0 +1,2 @@
+# we reject inline struct return type
+{ 'command': 'oops', 'returns': { 'a': 'str' } }
diff --git a/tests/qapi-schema/nested-struct-returns.out b/tests/qapi-schema/returns-dict.out
similarity index 100%
rename from tests/qapi-schema/nested-struct-returns.out
rename to tests/qapi-schema/returns-dict.out

From 8102307f51e68280ac965a140a87073d5c31e9a5 Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Sat, 27 Jun 2015 16:48:14 +0200
Subject: [PATCH 22/33] qapi-commands: Fix gen_err_check(e) for e and e !=
 'local_err'

gen_err_check() hard-codes 'local_err' instead of substituting the
argument.  Currently harmless, since all callers pass either None or
'local_err'.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 scripts/qapi-commands.py | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 69029f58fb..3965ca8e9a 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -29,14 +29,15 @@ def generate_command_decl(name, args, ret_type):
                  ret_type=c_type(ret_type), name=c_name(name),
                  args=arglist).strip()
 
-def gen_err_check(errvar):
-    if errvar:
-        return mcgen('''
-if (local_err) {
+def gen_err_check(err):
+    if not err:
+        return ''
+    return mcgen('''
+if (%(err)s) {
     goto out;
 }
-''')
-    return ''
+''',
+                 err=err)
 
 def gen_sync_call(name, args, ret_type):
     ret = ""

From e02bca281c82f874d84578af4deea46142232115 Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Sat, 27 Jun 2015 17:21:12 +0200
Subject: [PATCH 23/33] qapi-commands: Inline gen_marshal_output_call()

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 scripts/qapi-commands.py | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 3965ca8e9a..6de5229694 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -57,19 +57,15 @@ def gen_sync_call(name, args, ret_type):
                 name=c_name(name), args=arglist, retval=retval).rstrip()
     if ret_type:
         ret += "\n" + gen_err_check('local_err')
-        ret += "\n" + mcgen('''
-%(marshal_output_call)s
+        ret += mcgen('''
+
+qmp_marshal_output_%(c_name)s(retval, ret, &local_err);
 ''',
-                            marshal_output_call=gen_marshal_output_call(name, ret_type)).rstrip()
+                            c_name=c_name(name))
     pop_indent()
     return ret.rstrip()
 
 
-def gen_marshal_output_call(name, ret_type):
-    if not ret_type:
-        return ""
-    return "qmp_marshal_output_%s(retval, ret, &local_err);" % c_name(name)
-
 def gen_visitor_input_containers_decl(args):
     ret = ""
 

From 1f9a7a1a5862ad224aa86f9b4c046248ffc27aa3 Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Sat, 27 Jun 2015 17:49:34 +0200
Subject: [PATCH 24/33] qapi-commands: Don't feed output of mcgen() to mcgen()
 again

Multiple passes through mcgen() is prone to produce unwanted blank
lines, which we then combat by sprinkling .rstrip() on top.  Just
don't do it.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 scripts/qapi-commands.py | 52 ++++++++++++++++------------------------
 1 file changed, 21 insertions(+), 31 deletions(-)

diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 6de5229694..cfbd59c84e 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -27,7 +27,7 @@ def generate_command_decl(name, args, ret_type):
 %(ret_type)s qmp_%(name)s(%(args)sError **errp);
 ''',
                  ret_type=c_type(ret_type), name=c_name(name),
-                 args=arglist).strip()
+                 args=arglist)
 
 def gen_err_check(err):
     if not err:
@@ -52,19 +52,17 @@ def gen_sync_call(name, args, ret_type):
     push_indent()
     ret = mcgen('''
 %(retval)sqmp_%(name)s(%(args)s&local_err);
-
 ''',
-                name=c_name(name), args=arglist, retval=retval).rstrip()
+                name=c_name(name), args=arglist, retval=retval)
     if ret_type:
-        ret += "\n" + gen_err_check('local_err')
+        ret += gen_err_check('local_err')
         ret += mcgen('''
 
 qmp_marshal_output_%(c_name)s(retval, ret, &local_err);
 ''',
                             c_name=c_name(name))
     pop_indent()
-    return ret.rstrip()
-
+    return ret
 
 def gen_visitor_input_containers_decl(args):
     ret = ""
@@ -78,7 +76,7 @@ Visitor *v;
 ''')
     pop_indent()
 
-    return ret.rstrip()
+    return ret
 
 def gen_visitor_input_vars_decl(args):
     ret = ""
@@ -101,7 +99,7 @@ bool has_%(argname)s = false;
                          argname=c_name(argname), argtype=c_type(argtype))
 
     pop_indent()
-    return ret.rstrip()
+    return ret
 
 def gen_visitor_input_block(args, dealloc=False):
     ret = ""
@@ -155,7 +153,7 @@ visit_type_%(visitor)s(v, &%(c_name)s, "%(name)s", %(errp)s);
 qapi_dealloc_visitor_cleanup(md);
 ''')
     pop_indent()
-    return ret.rstrip()
+    return ret
 
 def gen_marshal_output(name, ret_type):
     if not ret_type:
@@ -217,26 +215,17 @@ def gen_marshal_input(name, args, ret_type, middle_mode):
                      retval=retval)
 
     if len(args) > 0:
-        ret += mcgen('''
-%(visitor_input_containers_decl)s
-%(visitor_input_vars_decl)s
-
-%(visitor_input_block)s
-
-''',
-                     visitor_input_containers_decl=gen_visitor_input_containers_decl(args),
-                     visitor_input_vars_decl=gen_visitor_input_vars_decl(args),
-                     visitor_input_block=gen_visitor_input_block(args))
+        ret += gen_visitor_input_containers_decl(args)
+        ret += gen_visitor_input_vars_decl(args) + '\n'
+        ret += gen_visitor_input_block(args) + '\n'
     else:
         ret += mcgen('''
 
     (void)args;
 ''')
 
-    ret += mcgen('''
-%(sync_call)s
-''',
-                 sync_call=gen_sync_call(name, args, ret_type))
+    ret += gen_sync_call(name, args, ret_type)
+
     if re.search('^ *goto out\\;', ret, re.MULTILINE):
         ret += mcgen('''
 
@@ -244,11 +233,11 @@ out:
 ''')
     ret += mcgen('''
     error_propagate(errp, local_err);
-%(visitor_input_block_cleanup)s
+''')
+    ret += gen_visitor_input_block(args, dealloc=True)
+    ret += mcgen('''
 }
-''',
-                 visitor_input_block_cleanup=gen_visitor_input_block(args,
-                                                                     dealloc=True))
+''')
     return ret
 
 def gen_registry(commands):
@@ -268,12 +257,13 @@ qmp_register_command("%(name)s", qmp_marshal_input_%(c_name)s, %(opts)s);
     ret = mcgen('''
 static void qmp_init_marshal(void)
 {
-%(registry)s
+''')
+    ret += registry
+    ret += mcgen('''
 }
 
 qapi_init(qmp_init_marshal);
-''',
-                registry=registry.rstrip())
+''')
     return ret
 
 middle_mode = False
@@ -353,7 +343,7 @@ for cmd in commands:
         arglist = cmd['data']
     if cmd.has_key('returns'):
         ret_type = cmd['returns']
-    ret = generate_command_decl(cmd['command'], arglist, ret_type) + "\n"
+    ret = generate_command_decl(cmd['command'], arglist, ret_type)
     fdecl.write(ret)
     if ret_type:
         ret = gen_marshal_output(cmd['command'], ret_type) + "\n"

From 3f99144cd9afbf51a7fbddf20b921402c2d4f68c Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Fri, 31 Jul 2015 18:51:18 +0200
Subject: [PATCH 25/33] qapi-commands: Drop useless initialization

In generated command handlers, the assignment to retval dominates its
only use.  Therefore, its initialization is useless.  Drop it.

Suggested-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 docs/qapi-code-gen.txt   | 2 +-
 scripts/qapi-commands.py | 8 ++------
 2 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index a253e27d5f..7cb852e614 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -742,7 +742,7 @@ Example:
     static void qmp_marshal_input_my_command(QDict *args, QObject **ret, Error **errp)
     {
         Error *local_err = NULL;
-        UserDefOne *retval = NULL;
+        UserDefOne *retval;
         QmpInputVisitor *mi = qmp_input_visitor_new_strict(QOBJECT(args));
         QapiDeallocVisitor *md;
         Visitor *v;
diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index cfbd59c84e..8bf84a77dd 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -205,14 +205,10 @@ def gen_marshal_input(name, args, ret_type, middle_mode):
                 header=hdr)
 
     if ret_type:
-        if is_c_ptr(ret_type):
-            retval = "    %s retval = NULL;" % c_type(ret_type)
-        else:
-            retval = "    %s retval;" % c_type(ret_type)
         ret += mcgen('''
-%(retval)s
+    %(c_type)s retval;
 ''',
-                     retval=retval)
+                     c_type=c_type(ret_type))
 
     if len(args) > 0:
         ret += gen_visitor_input_containers_decl(args)

From 3a864e7c52af15017d5082a9ee39a7919f46d2b5 Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Wed, 1 Jul 2015 16:55:15 +0200
Subject: [PATCH 26/33] qapi: Generated code cleanup

Clean up white-space, brace placement, and superfluous #ifdef
QAPI_TYPES_BUILTIN_CLEANUP_DEF.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 docs/qapi-code-gen.txt   | 12 +++-----
 scripts/qapi-commands.py |  1 +
 scripts/qapi-event.py    |  3 +-
 scripts/qapi-types.py    | 66 +++++++++++++++++++---------------------
 scripts/qapi-visit.py    |  1 +
 5 files changed, 39 insertions(+), 44 deletions(-)

diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index 7cb852e614..b4d4a0176b 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -568,7 +568,6 @@ Example:
         visit_type_UserDefOne(v, &obj, NULL, NULL);
         qapi_dealloc_visitor_cleanup(md);
     }
-
     $ cat qapi-generated/example-qapi-types.h
 [Uninteresting stuff omitted...]
 
@@ -579,8 +578,7 @@ Example:
 
     typedef struct UserDefOne UserDefOne;
 
-    typedef struct UserDefOneList
-    {
+    typedef struct UserDefOneList {
         union {
             UserDefOne *value;
             uint64_t padding;
@@ -588,10 +586,10 @@ Example:
         struct UserDefOneList *next;
     } UserDefOneList;
 
+
 [Functions on built-in types omitted...]
 
-    struct UserDefOne
-    {
+    struct UserDefOne {
         int64_t integer;
         char *string;
     };
@@ -629,6 +627,7 @@ Example:
     static void visit_type_UserDefOne_fields(Visitor *m, UserDefOne **obj, Error **errp)
     {
         Error *err = NULL;
+
         visit_type_int(m, &(*obj)->integer, "integer", &err);
         if (err) {
             goto out;
@@ -842,8 +841,7 @@ Example:
     void qapi_event_send_my_event(Error **errp);
 
     extern const char *example_QAPIEvent_lookup[];
-    typedef enum example_QAPIEvent
-    {
+    typedef enum example_QAPIEvent {
         EXAMPLE_QAPI_EVENT_MY_EVENT = 0,
         EXAMPLE_QAPI_EVENT_MAX = 1,
     } example_QAPIEvent;
diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 8bf84a77dd..890ce5db92 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -218,6 +218,7 @@ def gen_marshal_input(name, args, ret_type, middle_mode):
         ret += mcgen('''
 
     (void)args;
+
 ''')
 
     ret += gen_sync_call(name, args, ret_type)
diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
index 88b0620d00..7f238df5b2 100644
--- a/scripts/qapi-event.py
+++ b/scripts/qapi-event.py
@@ -167,8 +167,7 @@ extern const char *%(event_enum_name)s_lookup[];
                         event_enum_name = event_enum_name)
 
     enum_decl = mcgen('''
-typedef enum %(event_enum_name)s
-{
+typedef enum %(event_enum_name)s {
 ''',
                       event_enum_name = event_enum_name)
 
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 8444f9836a..f2428f3807 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -15,8 +15,7 @@ from qapi import *
 def generate_fwd_builtin(name):
     return mcgen('''
 
-typedef struct %(name)sList
-{
+typedef struct %(name)sList {
     union {
         %(type)s value;
         uint64_t padding;
@@ -32,8 +31,7 @@ def generate_fwd_struct(name):
 
 typedef struct %(name)s %(name)s;
 
-typedef struct %(name)sList
-{
+typedef struct %(name)sList {
     union {
         %(name)s *value;
         uint64_t padding;
@@ -45,8 +43,8 @@ typedef struct %(name)sList
 
 def generate_fwd_enum_struct(name):
     return mcgen('''
-typedef struct %(name)sList
-{
+
+typedef struct %(name)sList {
     union {
         %(name)s value;
         uint64_t padding;
@@ -79,8 +77,8 @@ def generate_struct(expr):
     base = expr.get('base')
 
     ret = mcgen('''
-struct %(name)s
-{
+
+struct %(name)s {
 ''',
           name=c_name(structname))
 
@@ -105,7 +103,8 @@ struct %(name)s
 
 def generate_enum_lookup(name, values):
     ret = mcgen('''
-const char * const %(name)s_lookup[] = {
+
+const char *const %(name)s_lookup[] = {
 ''',
                 name=c_name(name))
     for value in values:
@@ -119,7 +118,6 @@ const char * const %(name)s_lookup[] = {
     ret += mcgen('''
     [%(max_index)s] = NULL,
 };
-
 ''',
         max_index=max_index)
     return ret
@@ -127,13 +125,14 @@ const char * const %(name)s_lookup[] = {
 def generate_enum(name, values):
     name = c_name(name)
     lookup_decl = mcgen('''
-extern const char * const %(name)s_lookup[];
+
+extern const char *const %(name)s_lookup[];
 ''',
                 name=name)
 
     enum_decl = mcgen('''
-typedef enum %(name)s
-{
+
+typedef enum %(name)s {
 ''',
                 name=name)
 
@@ -155,7 +154,7 @@ typedef enum %(name)s
 ''',
                  name=name)
 
-    return lookup_decl + enum_decl
+    return enum_decl + lookup_decl
 
 def generate_alternate_qtypes(expr):
 
@@ -163,6 +162,7 @@ def generate_alternate_qtypes(expr):
     members = expr['data']
 
     ret = mcgen('''
+
 const int %(name)s_qtypes[QTYPE_MAX] = {
 ''',
                 name=c_name(name))
@@ -198,8 +198,8 @@ def generate_union(expr, meta):
         discriminator_type_name = '%sKind' % (name)
 
     ret = mcgen('''
-struct %(name)s
-{
+
+struct %(name)s {
 ''',
                 name=name)
     if base:
@@ -328,14 +328,12 @@ fdef.write(mcgen('''
 #include "qapi/dealloc-visitor.h"
 #include "%(prefix)sqapi-types.h"
 #include "%(prefix)sqapi-visit.h"
-
 ''',
                  prefix=prefix))
 
 fdecl.write(mcgen('''
 #include <stdbool.h>
 #include <stdint.h>
-
 '''))
 
 exprs = parse_schema(input_file)
@@ -346,22 +344,22 @@ for typename in builtin_types.keys():
 fdecl.write(guardend("QAPI_TYPES_BUILTIN_STRUCT_DECL"))
 
 for expr in exprs:
-    ret = "\n"
+    ret = ""
     if expr.has_key('struct'):
         ret += generate_fwd_struct(expr['struct'])
     elif expr.has_key('enum'):
-        ret += generate_enum(expr['enum'], expr['data']) + "\n"
+        ret += generate_enum(expr['enum'], expr['data'])
         ret += generate_fwd_enum_struct(expr['enum'])
         fdef.write(generate_enum_lookup(expr['enum'], expr['data']))
     elif expr.has_key('union'):
-        ret += generate_fwd_struct(expr['union']) + "\n"
+        ret += generate_fwd_struct(expr['union'])
         enum_define = discriminator_find_enum_define(expr)
         if not enum_define:
             ret += generate_enum('%sKind' % expr['union'], expr['data'].keys())
             fdef.write(generate_enum_lookup('%sKind' % expr['union'],
                                             expr['data'].keys()))
     elif expr.has_key('alternate'):
-        ret += generate_fwd_struct(expr['alternate']) + "\n"
+        ret += generate_fwd_struct(expr['alternate'])
         ret += generate_enum('%sKind' % expr['alternate'], expr['data'].keys())
         fdef.write(generate_enum_lookup('%sKind' % expr['alternate'],
                                         expr['data'].keys()))
@@ -381,34 +379,32 @@ fdecl.write(guardend("QAPI_TYPES_BUILTIN_CLEANUP_DECL"))
 # have the functions defined, so we use -b option to provide control
 # over these cases
 if do_builtins:
-    fdef.write(guardstart("QAPI_TYPES_BUILTIN_CLEANUP_DEF"))
     for typename in builtin_types.keys():
         fdef.write(generate_type_cleanup(typename + "List"))
-    fdef.write(guardend("QAPI_TYPES_BUILTIN_CLEANUP_DEF"))
 
 for expr in exprs:
-    ret = "\n"
+    ret = ""
     if expr.has_key('struct'):
         ret += generate_struct(expr) + "\n"
         ret += generate_type_cleanup_decl(expr['struct'] + "List")
-        fdef.write(generate_type_cleanup(expr['struct'] + "List") + "\n")
+        fdef.write(generate_type_cleanup(expr['struct'] + "List"))
         ret += generate_type_cleanup_decl(expr['struct'])
-        fdef.write(generate_type_cleanup(expr['struct']) + "\n")
+        fdef.write(generate_type_cleanup(expr['struct']))
     elif expr.has_key('union'):
-        ret += generate_union(expr, 'union')
+        ret += generate_union(expr, 'union') + "\n"
         ret += generate_type_cleanup_decl(expr['union'] + "List")
-        fdef.write(generate_type_cleanup(expr['union'] + "List") + "\n")
+        fdef.write(generate_type_cleanup(expr['union'] + "List"))
         ret += generate_type_cleanup_decl(expr['union'])
-        fdef.write(generate_type_cleanup(expr['union']) + "\n")
+        fdef.write(generate_type_cleanup(expr['union']))
     elif expr.has_key('alternate'):
-        ret += generate_union(expr, 'alternate')
+        ret += generate_union(expr, 'alternate') + "\n"
         ret += generate_type_cleanup_decl(expr['alternate'] + "List")
-        fdef.write(generate_type_cleanup(expr['alternate'] + "List") + "\n")
+        fdef.write(generate_type_cleanup(expr['alternate'] + "List"))
         ret += generate_type_cleanup_decl(expr['alternate'])
-        fdef.write(generate_type_cleanup(expr['alternate']) + "\n")
+        fdef.write(generate_type_cleanup(expr['alternate']))
     elif expr.has_key('enum'):
-        ret += generate_type_cleanup_decl(expr['enum'] + "List")
-        fdef.write(generate_type_cleanup(expr['enum'] + "List") + "\n")
+        ret += "\n" + generate_type_cleanup_decl(expr['enum'] + "List")
+        fdef.write(generate_type_cleanup(expr['enum'] + "List"))
     else:
         continue
     fdecl.write(ret)
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index eec5f1f4c5..3cd662bd6b 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -62,6 +62,7 @@ def generate_visit_struct_fields(name, members, base = None):
 static void visit_type_%(name)s_fields(Visitor *m, %(name)s **obj, Error **errp)
 {
     Error *err = NULL;
+
 ''',
                  name=c_name(name))
     push_indent()

From 65fbe125451da9421070ab03944c9600a264eefc Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Mon, 31 Aug 2015 15:37:42 +0200
Subject: [PATCH 27/33] qapi: Drop one of two "simple union must not have base"
 checks

The first check ensures the second one can't trigger.  Drop the first
one, because the second one is in a more logical place, and emits a
nicer error message.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 scripts/qapi.py                                   | 8 --------
 tests/qapi-schema/union-base-no-discriminator.err | 2 +-
 2 files changed, 1 insertion(+), 9 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 23c32fe3dd..197db77a9f 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -526,14 +526,6 @@ def check_union(expr, expr_info):
     members = expr['data']
     values = { 'MAX': '(automatic)' }
 
-    # If the object has a member 'base', its value must name a struct,
-    # and there must be a discriminator.
-    if base is not None:
-        if discriminator is None:
-            raise QAPIExprError(expr_info,
-                                "Union '%s' requires a discriminator to go "
-                                "along with base" %name)
-
     # Two types of unions, determined by discriminator.
 
     # With no discriminator it is a simple union.
diff --git a/tests/qapi-schema/union-base-no-discriminator.err b/tests/qapi-schema/union-base-no-discriminator.err
index fc8b79c459..8b7a24260f 100644
--- a/tests/qapi-schema/union-base-no-discriminator.err
+++ b/tests/qapi-schema/union-base-no-discriminator.err
@@ -1 +1 @@
-tests/qapi-schema/union-base-no-discriminator.json:11: Union 'TestUnion' requires a discriminator to go along with base
+tests/qapi-schema/union-base-no-discriminator.json:11: Simple union 'TestUnion' must not have a base

From 91f9816da4d505d379753896f3f7b6abb910324b Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Mon, 31 Aug 2015 15:47:55 +0200
Subject: [PATCH 28/33] tests/qapi-schema: Cover two more syntax errors

Syntax error coverage should now be complete.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 tests/Makefile                              | 1 +
 tests/qapi-schema/leading-comma-list.err    | 1 +
 tests/qapi-schema/leading-comma-list.exit   | 1 +
 tests/qapi-schema/leading-comma-list.json   | 2 ++
 tests/qapi-schema/leading-comma-list.out    | 0
 tests/qapi-schema/leading-comma-object.err  | 1 +
 tests/qapi-schema/leading-comma-object.exit | 1 +
 tests/qapi-schema/leading-comma-object.json | 2 ++
 tests/qapi-schema/leading-comma-object.out  | 0
 9 files changed, 9 insertions(+)
 create mode 100644 tests/qapi-schema/leading-comma-list.err
 create mode 100644 tests/qapi-schema/leading-comma-list.exit
 create mode 100644 tests/qapi-schema/leading-comma-list.json
 create mode 100644 tests/qapi-schema/leading-comma-list.out
 create mode 100644 tests/qapi-schema/leading-comma-object.err
 create mode 100644 tests/qapi-schema/leading-comma-object.exit
 create mode 100644 tests/qapi-schema/leading-comma-object.json
 create mode 100644 tests/qapi-schema/leading-comma-object.out

diff --git a/tests/Makefile b/tests/Makefile
index b8d445e955..597ca90703 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -237,6 +237,7 @@ check-qapi-schema-y := $(addprefix tests/qapi-schema/, \
 	missing-colon.json missing-comma-list.json missing-comma-object.json \
 	nested-struct-data.json non-objects.json \
 	qapi-schema-test.json quoted-structural-chars.json \
+	leading-comma-list.json leading-comma-object.json \
 	trailing-comma-list.json trailing-comma-object.json \
 	unclosed-list.json unclosed-object.json unclosed-string.json \
 	duplicate-key.json union-invalid-base.json union-bad-branch.json \
diff --git a/tests/qapi-schema/leading-comma-list.err b/tests/qapi-schema/leading-comma-list.err
new file mode 100644
index 0000000000..f5c870bb9c
--- /dev/null
+++ b/tests/qapi-schema/leading-comma-list.err
@@ -0,0 +1 @@
+tests/qapi-schema/leading-comma-list.json:2:13: Expected "{", "[", "]", string, boolean or "null"
diff --git a/tests/qapi-schema/leading-comma-list.exit b/tests/qapi-schema/leading-comma-list.exit
new file mode 100644
index 0000000000..d00491fd7e
--- /dev/null
+++ b/tests/qapi-schema/leading-comma-list.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/leading-comma-list.json b/tests/qapi-schema/leading-comma-list.json
new file mode 100644
index 0000000000..c5ba501590
--- /dev/null
+++ b/tests/qapi-schema/leading-comma-list.json
@@ -0,0 +1,2 @@
+{ 'enum': 'Status',
+  'data': [ , 'good', 'bad', 'ugly' ] }
diff --git a/tests/qapi-schema/leading-comma-list.out b/tests/qapi-schema/leading-comma-list.out
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/qapi-schema/leading-comma-object.err b/tests/qapi-schema/leading-comma-object.err
new file mode 100644
index 0000000000..f767b95544
--- /dev/null
+++ b/tests/qapi-schema/leading-comma-object.err
@@ -0,0 +1 @@
+tests/qapi-schema/leading-comma-object.json:1:3: Expected string or "}"
diff --git a/tests/qapi-schema/leading-comma-object.exit b/tests/qapi-schema/leading-comma-object.exit
new file mode 100644
index 0000000000..d00491fd7e
--- /dev/null
+++ b/tests/qapi-schema/leading-comma-object.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/leading-comma-object.json b/tests/qapi-schema/leading-comma-object.json
new file mode 100644
index 0000000000..c89023ff3b
--- /dev/null
+++ b/tests/qapi-schema/leading-comma-object.json
@@ -0,0 +1,2 @@
+{ , 'enum': 'Status',
+  'data': [ 'good', 'bad', 'ugly' ] }
diff --git a/tests/qapi-schema/leading-comma-object.out b/tests/qapi-schema/leading-comma-object.out
new file mode 100644
index 0000000000..e69de29bb2

From 10689e36eb99e99751497ac8cef2a946e9a3a850 Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Mon, 31 Aug 2015 17:17:42 +0200
Subject: [PATCH 29/33] tests/qapi-schema: Cover non-string, non-dictionary
 members

We always report "should be a dictionary" then.  This is misleading:
when allow_dict, it can be a dictionary or a type name string, else it
can only be a type name.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 tests/Makefile                               | 2 ++
 tests/qapi-schema/args-invalid.err           | 1 +
 tests/qapi-schema/args-invalid.exit          | 1 +
 tests/qapi-schema/args-invalid.json          | 3 +++
 tests/qapi-schema/args-invalid.out           | 0
 tests/qapi-schema/struct-data-invalid.err    | 1 +
 tests/qapi-schema/struct-data-invalid.exit   | 1 +
 tests/qapi-schema/struct-data-invalid.json   | 3 +++
 tests/qapi-schema/struct-data-invalid.out    | 0
 tests/qapi-schema/struct-member-invalid.err  | 1 +
 tests/qapi-schema/struct-member-invalid.exit | 1 +
 tests/qapi-schema/struct-member-invalid.json | 3 +++
 tests/qapi-schema/struct-member-invalid.out  | 0
 13 files changed, 17 insertions(+)
 create mode 100644 tests/qapi-schema/args-invalid.err
 create mode 100644 tests/qapi-schema/args-invalid.exit
 create mode 100644 tests/qapi-schema/args-invalid.json
 create mode 100644 tests/qapi-schema/args-invalid.out
 create mode 100644 tests/qapi-schema/struct-data-invalid.err
 create mode 100644 tests/qapi-schema/struct-data-invalid.exit
 create mode 100644 tests/qapi-schema/struct-data-invalid.json
 create mode 100644 tests/qapi-schema/struct-data-invalid.out
 create mode 100644 tests/qapi-schema/struct-member-invalid.err
 create mode 100644 tests/qapi-schema/struct-member-invalid.exit
 create mode 100644 tests/qapi-schema/struct-member-invalid.json
 create mode 100644 tests/qapi-schema/struct-member-invalid.out

diff --git a/tests/Makefile b/tests/Makefile
index 597ca90703..b128e288e4 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -229,12 +229,14 @@ check-qapi-schema-y := $(addprefix tests/qapi-schema/, \
 	redefined-type.json redefined-command.json redefined-builtin.json \
 	redefined-event.json command-int.json bad-data.json event-max.json \
 	type-bypass.json type-bypass-no-gen.json type-bypass-bad-gen.json \
+	args-invalid.json \
 	args-array-empty.json args-array-unknown.json args-int.json \
 	args-unknown.json args-member-unknown.json args-member-array.json \
 	args-member-array-bad.json args-alternate.json args-union.json \
 	returns-array-bad.json returns-int.json returns-dict.json \
 	returns-unknown.json returns-alternate.json returns-whitelist.json \
 	missing-colon.json missing-comma-list.json missing-comma-object.json \
+	struct-data-invalid.json struct-member-invalid.json \
 	nested-struct-data.json non-objects.json \
 	qapi-schema-test.json quoted-structural-chars.json \
 	leading-comma-list.json leading-comma-object.json \
diff --git a/tests/qapi-schema/args-invalid.err b/tests/qapi-schema/args-invalid.err
new file mode 100644
index 0000000000..6d0fa05c80
--- /dev/null
+++ b/tests/qapi-schema/args-invalid.err
@@ -0,0 +1 @@
+tests/qapi-schema/args-invalid.json:2: 'data' for command 'foo' should be a dictionary
diff --git a/tests/qapi-schema/args-invalid.exit b/tests/qapi-schema/args-invalid.exit
new file mode 100644
index 0000000000..d00491fd7e
--- /dev/null
+++ b/tests/qapi-schema/args-invalid.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/args-invalid.json b/tests/qapi-schema/args-invalid.json
new file mode 100644
index 0000000000..4a641350e3
--- /dev/null
+++ b/tests/qapi-schema/args-invalid.json
@@ -0,0 +1,3 @@
+# FIXME error "should be a dictionary" is misleading, type name is also fine
+{ 'command': 'foo',
+  'data': false }
diff --git a/tests/qapi-schema/args-invalid.out b/tests/qapi-schema/args-invalid.out
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/qapi-schema/struct-data-invalid.err b/tests/qapi-schema/struct-data-invalid.err
new file mode 100644
index 0000000000..a40e23aaea
--- /dev/null
+++ b/tests/qapi-schema/struct-data-invalid.err
@@ -0,0 +1 @@
+tests/qapi-schema/struct-data-invalid.json:2: 'data' for struct 'foo' should be a dictionary
diff --git a/tests/qapi-schema/struct-data-invalid.exit b/tests/qapi-schema/struct-data-invalid.exit
new file mode 100644
index 0000000000..d00491fd7e
--- /dev/null
+++ b/tests/qapi-schema/struct-data-invalid.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/struct-data-invalid.json b/tests/qapi-schema/struct-data-invalid.json
new file mode 100644
index 0000000000..b76de82552
--- /dev/null
+++ b/tests/qapi-schema/struct-data-invalid.json
@@ -0,0 +1,3 @@
+# FIXME error "should be a dictionary" is misleading, type name is also fine
+{ 'struct': 'foo',
+  'data': false }
diff --git a/tests/qapi-schema/struct-data-invalid.out b/tests/qapi-schema/struct-data-invalid.out
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/qapi-schema/struct-member-invalid.err b/tests/qapi-schema/struct-member-invalid.err
new file mode 100644
index 0000000000..9e4c7b85ff
--- /dev/null
+++ b/tests/qapi-schema/struct-member-invalid.err
@@ -0,0 +1 @@
+tests/qapi-schema/struct-member-invalid.json:2: Member 'a' of 'data' for struct 'foo' should be a dictionary
diff --git a/tests/qapi-schema/struct-member-invalid.exit b/tests/qapi-schema/struct-member-invalid.exit
new file mode 100644
index 0000000000..d00491fd7e
--- /dev/null
+++ b/tests/qapi-schema/struct-member-invalid.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/struct-member-invalid.json b/tests/qapi-schema/struct-member-invalid.json
new file mode 100644
index 0000000000..968e63cb8e
--- /dev/null
+++ b/tests/qapi-schema/struct-member-invalid.json
@@ -0,0 +1,3 @@
+# FIXME error message "should be a dictionary" is wrong, must be type name
+{ 'struct': 'foo',
+  'data': { 'a': false } }
diff --git a/tests/qapi-schema/struct-member-invalid.out b/tests/qapi-schema/struct-member-invalid.out
new file mode 100644
index 0000000000..e69de29bb2

From c6b71e5ae73802057d700e2419b80aef1651f213 Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Mon, 31 Aug 2015 17:28:52 +0200
Subject: [PATCH 30/33] qapi: Fix errors for non-string, non-dictionary members

Fixes the errors demonstrated by the previous commit.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 scripts/qapi.py                              | 10 ++++++----
 tests/qapi-schema/args-invalid.err           |  2 +-
 tests/qapi-schema/args-invalid.json          |  1 -
 tests/qapi-schema/struct-data-invalid.err    |  2 +-
 tests/qapi-schema/struct-data-invalid.json   |  1 -
 tests/qapi-schema/struct-member-invalid.err  |  2 +-
 tests/qapi-schema/struct-member-invalid.json |  1 -
 7 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 197db77a9f..36560590ba 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -462,13 +462,15 @@ def check_type(expr_info, source, value, allow_array = False,
                                 % (source, all_names[value], orig_value))
         return
 
-    # value is a dictionary, check that each member is okay
-    if not isinstance(value, OrderedDict):
-        raise QAPIExprError(expr_info,
-                            "%s should be a dictionary" % source)
     if not allow_dict:
         raise QAPIExprError(expr_info,
                             "%s should be a type name" % source)
+
+    if not isinstance(value, OrderedDict):
+        raise QAPIExprError(expr_info,
+                            "%s should be a dictionary or type name" % source)
+
+    # value is a dictionary, check that each member is okay
     for (key, arg) in value.items():
         check_name(expr_info, "Member of %s" % source, key,
                    allow_optional=allow_optional)
diff --git a/tests/qapi-schema/args-invalid.err b/tests/qapi-schema/args-invalid.err
index 6d0fa05c80..fe1e94975b 100644
--- a/tests/qapi-schema/args-invalid.err
+++ b/tests/qapi-schema/args-invalid.err
@@ -1 +1 @@
-tests/qapi-schema/args-invalid.json:2: 'data' for command 'foo' should be a dictionary
+tests/qapi-schema/args-invalid.json:1: 'data' for command 'foo' should be a dictionary or type name
diff --git a/tests/qapi-schema/args-invalid.json b/tests/qapi-schema/args-invalid.json
index 4a641350e3..db0981341b 100644
--- a/tests/qapi-schema/args-invalid.json
+++ b/tests/qapi-schema/args-invalid.json
@@ -1,3 +1,2 @@
-# FIXME error "should be a dictionary" is misleading, type name is also fine
 { 'command': 'foo',
   'data': false }
diff --git a/tests/qapi-schema/struct-data-invalid.err b/tests/qapi-schema/struct-data-invalid.err
index a40e23aaea..6644f4c2ad 100644
--- a/tests/qapi-schema/struct-data-invalid.err
+++ b/tests/qapi-schema/struct-data-invalid.err
@@ -1 +1 @@
-tests/qapi-schema/struct-data-invalid.json:2: 'data' for struct 'foo' should be a dictionary
+tests/qapi-schema/struct-data-invalid.json:1: 'data' for struct 'foo' should be a dictionary or type name
diff --git a/tests/qapi-schema/struct-data-invalid.json b/tests/qapi-schema/struct-data-invalid.json
index b76de82552..9adbc3bb6b 100644
--- a/tests/qapi-schema/struct-data-invalid.json
+++ b/tests/qapi-schema/struct-data-invalid.json
@@ -1,3 +1,2 @@
-# FIXME error "should be a dictionary" is misleading, type name is also fine
 { 'struct': 'foo',
   'data': false }
diff --git a/tests/qapi-schema/struct-member-invalid.err b/tests/qapi-schema/struct-member-invalid.err
index 9e4c7b85ff..69a326d450 100644
--- a/tests/qapi-schema/struct-member-invalid.err
+++ b/tests/qapi-schema/struct-member-invalid.err
@@ -1 +1 @@
-tests/qapi-schema/struct-member-invalid.json:2: Member 'a' of 'data' for struct 'foo' should be a dictionary
+tests/qapi-schema/struct-member-invalid.json:1: Member 'a' of 'data' for struct 'foo' should be a type name
diff --git a/tests/qapi-schema/struct-member-invalid.json b/tests/qapi-schema/struct-member-invalid.json
index 968e63cb8e..8f172f7a87 100644
--- a/tests/qapi-schema/struct-member-invalid.json
+++ b/tests/qapi-schema/struct-member-invalid.json
@@ -1,3 +1,2 @@
-# FIXME error message "should be a dictionary" is wrong, must be type name
 { 'struct': 'foo',
   'data': { 'a': false } }

From eddf817bd823a90df209dfbdc2a0b2ec33b7cb77 Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Mon, 31 Aug 2015 13:54:39 +0200
Subject: [PATCH 31/33] qapi: Simplify error reporting for array types

check_type() first checks and peels off the array type, then checks
the element type.  For two out of four error messages, it takes pains
to report errors for "array of T" instead of just T.  Odd.  Let's
examine the errors.

* Unknown element type, e.g.
  tests/qapi-schema/args-array-unknown.json:

      Member 'array' of 'data' for command 'oops' uses unknown type
      'array of NoSuchType'

  To make sense of this, you need to know that 'array of NoSuchType'
  refers to '[NoSuchType]'.  Easy enough.  However, simply reporting

      Member 'array' of 'data' for command 'oops' uses unknown type
      'NoSuchType'

  is at least as easy to understand.

* Element type's meta-type is inadmissible, e.g.
  tests/qapi-schema/returns-whitelist.json:

      'returns' for command 'no-way-this-will-get-whitelisted' cannot
      use built-in type 'array of int'

  'array of int' is technically not a built-in type, but that's
  pedantry.  However, simply reporting

      'returns' for command 'no-way-this-will-get-whitelisted' cannot
      use built-in type 'int'

  avoids the issue, and is at least as easy to understand.

* The remaining two errors are unreachable, because the array checking
  ensures that value is a string.

Thus, reporting some errors for "array of T" instead of just T works,
but doesn't really improve things.  Drop it.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 scripts/qapi.py                          | 6 ++----
 tests/qapi-schema/args-array-unknown.err | 2 +-
 tests/qapi-schema/returns-whitelist.err  | 2 +-
 3 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 36560590ba..ac0e45e2e6 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -426,7 +426,6 @@ def check_type(expr_info, source, value, allow_array = False,
                allow_dict = False, allow_optional = False,
                allow_star = False, allow_metas = []):
     global all_names
-    orig_value = value
 
     if value is None:
         return
@@ -444,7 +443,6 @@ def check_type(expr_info, source, value, allow_array = False,
                                 "%s: array type must contain single type name"
                                 % source)
         value = value[0]
-        orig_value = "array of %s" %value
 
     # Check if type name for value is okay
     if isinstance(value, str):
@@ -455,11 +453,11 @@ def check_type(expr_info, source, value, allow_array = False,
         if not value in all_names:
             raise QAPIExprError(expr_info,
                                 "%s uses unknown type '%s'"
-                                % (source, orig_value))
+                                % (source, value))
         if not all_names[value] in allow_metas:
             raise QAPIExprError(expr_info,
                                 "%s cannot use %s type '%s'"
-                                % (source, all_names[value], orig_value))
+                                % (source, all_names[value], value))
         return
 
     if not allow_dict:
diff --git a/tests/qapi-schema/args-array-unknown.err b/tests/qapi-schema/args-array-unknown.err
index 7834d11bc1..cd7a0f98d7 100644
--- a/tests/qapi-schema/args-array-unknown.err
+++ b/tests/qapi-schema/args-array-unknown.err
@@ -1 +1 @@
-tests/qapi-schema/args-array-unknown.json:2: Member 'array' of 'data' for command 'oops' uses unknown type 'array of NoSuchType'
+tests/qapi-schema/args-array-unknown.json:2: Member 'array' of 'data' for command 'oops' uses unknown type 'NoSuchType'
diff --git a/tests/qapi-schema/returns-whitelist.err b/tests/qapi-schema/returns-whitelist.err
index a41f019a52..f47c1ee7ca 100644
--- a/tests/qapi-schema/returns-whitelist.err
+++ b/tests/qapi-schema/returns-whitelist.err
@@ -1 +1 @@
-tests/qapi-schema/returns-whitelist.json:10: 'returns' for command 'no-way-this-will-get-whitelisted' cannot use built-in type 'array of int'
+tests/qapi-schema/returns-whitelist.json:10: 'returns' for command 'no-way-this-will-get-whitelisted' cannot use built-in type 'int'

From 94a3f0af388820d74a9d89d1a856d2baa448c696 Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Thu, 3 Sep 2015 10:18:06 +0200
Subject: [PATCH 32/33] docs/qapi-code-gen.txt: Fix QAPI schema examples

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 docs/qapi-code-gen.txt | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index b4d4a0176b..ff16df2456 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -300,7 +300,6 @@ an implicit C enum 'NameKind' is created, corresponding to the union
 the union can be named 'max', as this would collide with the implicit
 enum.  The value for each branch can be of any type.
 
-
 A flat union definition specifies a struct as its base, and
 avoids nesting on the wire.  All branches of the union must be
 complex types, and the top-level fields of the union dictionary on
@@ -314,7 +313,7 @@ adding a common field 'readonly', renaming the discriminator to
 something more applicable, and reducing the number of {} required on
 the wire:
 
- { 'enum': 'BlockdevDriver', 'data': [ 'raw', 'qcow2' ] }
+ { 'enum': 'BlockdevDriver', 'data': [ 'file', 'qcow2' ] }
  { 'struct': 'BlockdevCommonOptions',
    'data': { 'driver': 'BlockdevDriver', 'readonly': 'bool' } }
  { 'union': 'BlockdevOptions',
@@ -350,7 +349,7 @@ is identical on the wire to:
  { 'struct': 'Base', 'data': { 'type': 'Enum' } }
  { 'struct': 'Branch1', 'data': { 'data': 'str' } }
  { 'struct': 'Branch2', 'data': { 'data': 'int' } }
- { 'union': 'Flat': 'base': 'Base', 'discriminator': 'type',
+ { 'union': 'Flat', 'base': 'Base', 'discriminator': 'type',
    'data': { 'one': 'Branch1', 'two': 'Branch2' } }
 
 

From c4f498fe8532cdacc609262b104322911108df54 Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Thu, 3 Sep 2015 10:24:25 +0200
Subject: [PATCH 33/33] qapi: Generators crash when --output-dir isn't given,
 fix

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 scripts/qapi.py | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index ac0e45e2e6..817d824bea 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -1035,11 +1035,12 @@ def open_output(output_dir, do_c, do_h, prefix, c_file, h_file,
     c_file = output_dir + prefix + c_file
     h_file = output_dir + prefix + h_file
 
-    try:
-        os.makedirs(output_dir)
-    except os.error, e:
-        if e.errno != errno.EEXIST:
-            raise
+    if output_dir:
+        try:
+            os.makedirs(output_dir)
+        except os.error, e:
+            if e.errno != errno.EEXIST:
+                raise
 
     def maybe_open(really, name, opt):
         if really: