[Qemu-devel] [PULL 00/33] qapi: Another round of fixes and cleanups

[Qemu-devel] [PULL 00/33] qapi: Another round of fixes and cleanups

[Markus Armbruster]

The following changes since commit b041066421e8dcc7d080dfcfd83551c9c9f24ade:

  Merge remote-tracking branch 'remotes/stefanha/tags/tracing-pull-request' into staging (2015-09-03 16:17:28 +0100)

are available in the git repository at:

  git://repo.or.cz/qemu/armbru.git tags/pull-qapi-2015-09-04

for you to fetch changes up to c4f498fe8532cdacc609262b104322911108df54:

  qapi: Generators crash when --output-dir isn't given, fix (2015-09-04 15:47:16 +0200)

----------------------------------------------------------------
qapi: Another round of fixes and cleanups

----------------------------------------------------------------
Eric Blake (2):
      qapi: Document that input visitor semantics are prone to leaks
      qapi: Document shortcoming with union 'data' branch

Markus Armbruster (31):
      qapi: Clarify docs on including the same file multiple times
      qapi: Clean up cgen() and mcgen()
      qapi: Simplify guardname()
      qapi-event: Clean up how name of enum QAPIEvent is made
      qapi: Reject -p arguments that break qapi-event.py
      qapi: Drop unused and useless parameters and variables
      qapi: Fix generated code when flat union has member 'kind'
      qapi: Generate a nicer struct for flat unions
      qapi-visit: Fix generated code when schema has forward refs
      qapi-visit: Replace list implicit_structs by set
      qapi-visit: Fix two name arguments passed to visitors
      tests/qapi-schema: Document alternate's enum lacks visit function
      tests/qapi-schema: Document events with base don't work
      qapi: Document flaws in checking of names
      tests/qapi-schema: Restore test case for flat union base bug
      tests/qapi-schema: Rename tests from data- to args-
      qapi-tests: New tests for union, alternate command arguments
      qapi: Fix to reject union command and event arguments
      qapi: Command returning anonymous type doesn't work, outlaw
      qapi-commands: Fix gen_err_check(e) for e and e != 'local_err'
      qapi-commands: Inline gen_marshal_output_call()
      qapi-commands: Don't feed output of mcgen() to mcgen() again
      qapi-commands: Drop useless initialization
      qapi: Generated code cleanup
      qapi: Drop one of two "simple union must not have base" checks
      tests/qapi-schema: Cover two more syntax errors
      tests/qapi-schema: Cover non-string, non-dictionary members
      qapi: Fix errors for non-string, non-dictionary members
      qapi: Simplify error reporting for array types
      docs/qapi-code-gen.txt: Fix QAPI schema examples
      qapi: Generators crash when --output-dir isn't given, fix

 docs/qapi-code-gen.txt                             |  53 +++++-----
 scripts/qapi-commands.py                           | 109 +++++++++-----------
 scripts/qapi-event.py                              |   6 +-
 scripts/qapi-types.py                              | 110 +++++++++++----------
 scripts/qapi-visit.py                              |  93 ++++++++++-------
 scripts/qapi.py                                    |  81 +++++++--------
 tests/Makefile                                     |  12 ++-
 tests/qapi-schema/args-alternate.err               |   1 +
 ...ted-struct-returns.exit => args-alternate.exit} |   0
 tests/qapi-schema/args-alternate.json              |   3 +
 ...ested-struct-returns.out => args-alternate.out} |   0
 .../{data-array-empty.err => args-array-empty.err} |   2 +-
 .../{data-unknown.exit => args-array-empty.exit}   |   0
 ...data-array-empty.json => args-array-empty.json} |   0
 .../{data-unknown.out => args-array-empty.out}     |   0
 tests/qapi-schema/args-array-unknown.err           |   1 +
 ...member-unknown.exit => args-array-unknown.exit} |   0
 ...-array-unknown.json => args-array-unknown.json} |   0
 ...a-member-unknown.out => args-array-unknown.out} |   0
 tests/qapi-schema/args-int.err                     |   1 +
 .../{data-member-array-bad.exit => args-int.exit}  |   0
 tests/qapi-schema/{data-int.json => args-int.json} |   0
 .../{data-member-array.err => args-int.out}        |   0
 tests/qapi-schema/args-invalid.err                 |   1 +
 .../{data-int.exit => args-invalid.exit}           |   0
 tests/qapi-schema/args-invalid.json                |   2 +
 ...{data-member-array-bad.out => args-invalid.out} |   0
 ...ber-array-bad.err => args-member-array-bad.err} |   2 +-
 ...ray-unknown.exit => args-member-array-bad.exit} |   0
 ...r-array-bad.json => args-member-array-bad.json} |   0
 .../{data-int.out => args-member-array-bad.out}    |   0
 ...ata-array-unknown.out => args-member-array.err} |   0
 ...ta-member-array.exit => args-member-array.exit} |   0
 ...ta-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 +
 ...a-array-empty.exit => args-member-unknown.exit} |   0
 ...ember-unknown.json => args-member-unknown.json} |   0
 ...ata-array-empty.out => args-member-unknown.out} |   0
 tests/qapi-schema/args-union.err                   |   1 +
 tests/qapi-schema/args-union.exit                  |   1 +
 tests/qapi-schema/args-union.json                  |   4 +
 tests/qapi-schema/args-union.out                   |   0
 tests/qapi-schema/args-unknown.err                 |   1 +
 tests/qapi-schema/args-unknown.exit                |   1 +
 .../{data-unknown.json => args-unknown.json}       |   0
 tests/qapi-schema/args-unknown.out                 |   0
 tests/qapi-schema/command-int.json                 |   3 +-
 tests/qapi-schema/data-array-unknown.err           |   1 -
 tests/qapi-schema/data-int.err                     |   1 -
 tests/qapi-schema/data-member-unknown.err          |   1 -
 tests/qapi-schema/data-unknown.err                 |   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
 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/qapi-schema-test.json            |  39 +++++---
 tests/qapi-schema/qapi-schema-test.out             |  14 +--
 tests/qapi-schema/returns-dict.err                 |   1 +
 tests/qapi-schema/returns-dict.exit                |   1 +
 tests/qapi-schema/returns-dict.json                |   2 +
 tests/qapi-schema/returns-dict.out                 |   0
 tests/qapi-schema/returns-whitelist.err            |   2 +-
 tests/qapi-schema/struct-data-invalid.err          |   1 +
 tests/qapi-schema/struct-data-invalid.exit         |   1 +
 tests/qapi-schema/struct-data-invalid.json         |   2 +
 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       |   2 +
 tests/qapi-schema/struct-member-invalid.out        |   0
 tests/qapi-schema/union-base-no-discriminator.err  |   2 +-
 tests/test-qmp-event.c                             |   2 +-
 tests/test-qmp-input-visitor.c                     |   4 +-
 tests/test-qmp-output-visitor.c                    |   2 +-
 82 files changed, 317 insertions(+), 268 deletions(-)
 create mode 100644 tests/qapi-schema/args-alternate.err
 rename tests/qapi-schema/{nested-struct-returns.exit => args-alternate.exit} (100%)
 create mode 100644 tests/qapi-schema/args-alternate.json
 rename tests/qapi-schema/{nested-struct-returns.out => args-alternate.out} (100%)
 rename tests/qapi-schema/{data-array-empty.err => args-array-empty.err} (50%)
 rename tests/qapi-schema/{data-unknown.exit => args-array-empty.exit} (100%)
 rename tests/qapi-schema/{data-array-empty.json => args-array-empty.json} (100%)
 rename tests/qapi-schema/{data-unknown.out => args-array-empty.out} (100%)
 create mode 100644 tests/qapi-schema/args-array-unknown.err
 rename tests/qapi-schema/{data-member-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-member-unknown.out => args-array-unknown.out} (100%)
 create mode 100644 tests/qapi-schema/args-int.err
 rename tests/qapi-schema/{data-member-array-bad.exit => args-int.exit} (100%)
 rename tests/qapi-schema/{data-int.json => args-int.json} (100%)
 rename tests/qapi-schema/{data-member-array.err => args-int.out} (100%)
 create mode 100644 tests/qapi-schema/args-invalid.err
 rename tests/qapi-schema/{data-int.exit => args-invalid.exit} (100%)
 create mode 100644 tests/qapi-schema/args-invalid.json
 rename tests/qapi-schema/{data-member-array-bad.out => args-invalid.out} (100%)
 rename tests/qapi-schema/{data-member-array-bad.err => args-member-array-bad.err} (52%)
 rename tests/qapi-schema/{data-array-unknown.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-int.out => args-member-array-bad.out} (100%)
 rename tests/qapi-schema/{data-array-unknown.out => 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-array-empty.exit => args-member-unknown.exit} (100%)
 rename tests/qapi-schema/{data-member-unknown.json => args-member-unknown.json} (100%)
 rename tests/qapi-schema/{data-array-empty.out => args-member-unknown.out} (100%)
 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
 create mode 100644 tests/qapi-schema/args-unknown.err
 create mode 100644 tests/qapi-schema/args-unknown.exit
 rename tests/qapi-schema/{data-unknown.json => args-unknown.json} (100%)
 create mode 100644 tests/qapi-schema/args-unknown.out
 delete mode 100644 tests/qapi-schema/data-array-unknown.err
 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
 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
 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
 create mode 100644 tests/qapi-schema/returns-dict.exit
 create mode 100644 tests/qapi-schema/returns-dict.json
 create mode 100644 tests/qapi-schema/returns-dict.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

-- 
2.4.3

[Qemu-devel] [PULL 01/33] qapi: Clarify docs on including the same file multiple times

[Markus Armbruster]

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 61b5be4..e7e7281 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)
-- 
2.4.3

[Qemu-devel] [PULL 02/33] qapi: Clean up cgen() and mcgen()

[Markus Armbruster]

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          | 19 ++++++++++++-------
 2 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index ca22acc..ce51408 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 06d7fc2..e656beb 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -943,16 +943,21 @@ 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'
-
-def mcgen(code, **kwds):
-    raw = cgen('\n'.join(code.split('\n')[1:-1]), **kwds)
+    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):
+    if code[0] == '\n':
+        code = code[1:]
+    return cgen(code, **kwds)
+
 def basename(filename):
     return filename.split("/")[-1]
 
-- 
2.4.3

[Qemu-devel] [PULL 03/33] qapi: Simplify guardname()

[Markus Armbruster]

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 e656beb..ba11c54 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)
 
-- 
2.4.3

[Qemu-devel] [PULL 04/33] qapi-event: Clean up how name of enum QAPIEvent is made

[Markus Armbruster]

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 e7e7281..c2ac21c 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 56bc602..cc74f4d 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 1ee40e1..28f146d 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;
-- 
2.4.3

[Qemu-devel] [PULL 05/33] qapi: Reject -p arguments that break qapi-event.py

[Markus Armbruster]

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 ba11c54..bc3f4d3 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 + "/"
-- 
2.4.3

[Qemu-devel] [PULL 06/33] qapi: Drop unused and useless parameters and variables

[Markus Armbruster]

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 ce51408..69029f5 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 cc74f4d..88b0620 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 e6eb4b6..4902440 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 5b99336..e8ee268 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 bc3f4d3..e7c814d 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")
-- 
2.4.3

[Qemu-devel] [PULL 07/33] qapi: Fix generated code when flat union has member 'kind'

[Markus Armbruster]

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 4902440..ac8dad3 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 e8ee268..4ec79a6 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 b961953..b7a87ee 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 87ba350..338ada0 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; */
-- 
2.4.3

[Qemu-devel] [PULL 08/33] qapi: Generate a nicer struct for flat unions

[Markus Armbruster]

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 ac8dad3..82141cd 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':
-- 
2.4.3

[Qemu-devel] [PULL 09/33] qapi-visit: Fix generated code when schema has forward refs

[Markus Armbruster]

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 | 30 +++++++++++++++++-------------
 tests/qapi-schema/qapi-schema-test.out  | 10 +++++-----
 3 files changed, 36 insertions(+), 19 deletions(-)

diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 4ec79a6..b3a308f 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 c7eaa86..ccadb13 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': 'UserDefZero',
-  'data': { 'integer': 'int' } }
-
 { 'struct': 'UserDefOne',
-  'base': 'UserDefZero',
+  'base': 'UserDefZero',        # intentional forward reference
   'data': { 'string': 'str', '*enum1': 'EnumOne' } }
 
+{ 'struct': 'UserDefZero',
+  'data': { 'integer': 'int' } }
+
 { 'struct': 'UserDefTwoDictDict',
   'data': { 'userdef': 'UserDefOne', 'string': 'str' } }
 
@@ -33,29 +33,33 @@
 { 'struct': 'UserDefB',
   'data': { 'integer': 'int' } }
 
-{ 'struct': 'UserDefC',
-  'data': { 'string1': 'str', 'string2': 'str' } }
-
-{ 'struct': 'UserDefUnionBase',
-  'data': { 'string': 'str', 'enum1': 'EnumOne' } }
-
 { 'union': 'UserDefFlatUnion',
-  'base': 'UserDefUnionBase',
+  'base': 'UserDefUnionBase',   # intentional forward reference
   'discriminator': 'enum1',
-  'data': { 'value1' : 'UserDefA', 'value2' : 'UserDefB', 'value3' : 'UserDefB' } }
+  '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' } }
+
 # 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 cf0ccc4..a4291db 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')]))]),
-- 
2.4.3

[Qemu-devel] [PULL 10/33] qapi-visit: Replace list implicit_structs by set

[Markus Armbruster]

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 b3a308f..9fc040e 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
-- 
2.4.3

[Qemu-devel] [PULL 11/33] qapi-visit: Fix two name arguments passed to visitors

[Markus Armbruster]

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 9fc040e..73f136f 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('''
-- 
2.4.3

[Qemu-devel] [PULL 12/33] tests/qapi-schema: Document alternate's enum lacks visit function

[Markus Armbruster]

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 ccadb13..8337ba9 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' } }
-- 
2.4.3

[Qemu-devel] [PULL 13/33] tests/qapi-schema: Document events with base don't work

[Markus Armbruster]

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 8337ba9..829dd30 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' },
-- 
2.4.3

[Qemu-devel] [PULL 14/33] qapi: Document that input visitor semantics are prone to leaks

[Markus Armbruster]

From: Eric Blake <eblake@redhat.com>

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 73f136f..eec5f1f 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 b7a87ee..a5cfefa 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);
-- 
2.4.3

[Qemu-devel] [PULL 15/33] qapi: Document shortcoming with union 'data' branch

[Markus Armbruster]

From: Eric Blake <eblake@redhat.com>

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 82141cd..8444f98 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;
-- 
2.4.3

[Qemu-devel] [PULL 16/33] qapi: Document flaws in checking of names

[Markus Armbruster]

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 e7c814d..4879982 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"
-- 
2.4.3

[Qemu-devel] [PULL 17/33] tests/qapi-schema: Restore test case for flat union base bug

[Markus Armbruster]

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 829dd30..a9e5aab 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 a4291db..b0b7187 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')]))]),
-- 
2.4.3

[Qemu-devel] [PULL 18/33] tests/qapi-schema: Rename tests from data- to args-

[Markus Armbruster]

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 +++---
 tests/qapi-schema/{data-array-empty.err => args-array-empty.err}    | 2 +-
 tests/qapi-schema/{data-unknown.exit => args-array-empty.exit}      | 0
 tests/qapi-schema/{data-array-empty.json => args-array-empty.json}  | 0
 tests/qapi-schema/{data-unknown.out => args-array-empty.out}        | 0
 .../qapi-schema/{data-array-unknown.err => args-array-unknown.err}  | 2 +-
 .../{data-member-unknown.exit => args-array-unknown.exit}           | 0
 .../{data-array-unknown.json => args-array-unknown.json}            | 0
 .../qapi-schema/{data-member-unknown.out => args-array-unknown.out} | 0
 tests/qapi-schema/args-int.err                                      | 1 +
 tests/qapi-schema/{data-member-array-bad.exit => args-int.exit}     | 0
 tests/qapi-schema/{data-int.json => args-int.json}                  | 0
 tests/qapi-schema/{data-member-array.err => args-int.out}           | 0
 .../{data-member-array-bad.err => args-member-array-bad.err}        | 2 +-
 tests/qapi-schema/{data-int.exit => args-member-array-bad.exit}     | 0
 .../{data-member-array-bad.json => args-member-array-bad.json}      | 0
 .../{data-member-array-bad.out => args-member-array-bad.out}        | 0
 tests/qapi-schema/{data-int.out => args-member-array.err}           | 0
 .../qapi-schema/{data-member-array.exit => args-member-array.exit}  | 0
 .../qapi-schema/{data-member-array.json => args-member-array.json}  | 0
 tests/qapi-schema/{data-member-array.out => args-member-array.out}  | 0
 tests/qapi-schema/args-member-unknown.err                           | 1 +
 .../{data-array-unknown.exit => args-member-unknown.exit}           | 0
 .../{data-member-unknown.json => args-member-unknown.json}          | 0
 .../qapi-schema/{data-array-unknown.out => args-member-unknown.out} | 0
 tests/qapi-schema/args-unknown.err                                  | 1 +
 tests/qapi-schema/{data-array-empty.exit => args-unknown.exit}      | 0
 tests/qapi-schema/{data-unknown.json => args-unknown.json}          | 0
 tests/qapi-schema/{data-array-empty.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-unknown.exit => args-array-empty.exit} (100%)
 rename tests/qapi-schema/{data-array-empty.json => args-array-empty.json} (100%)
 rename tests/qapi-schema/{data-unknown.out => args-array-empty.out} (100%)
 rename tests/qapi-schema/{data-array-unknown.err => args-array-unknown.err} (50%)
 rename tests/qapi-schema/{data-member-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-member-unknown.out => args-array-unknown.out} (100%)
 create mode 100644 tests/qapi-schema/args-int.err
 rename tests/qapi-schema/{data-member-array-bad.exit => args-int.exit} (100%)
 rename tests/qapi-schema/{data-int.json => args-int.json} (100%)
 rename tests/qapi-schema/{data-member-array.err => args-int.out} (100%)
 rename tests/qapi-schema/{data-member-array-bad.err => args-member-array-bad.err} (52%)
 rename tests/qapi-schema/{data-int.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-int.out => 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-array-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-array-unknown.out => args-member-unknown.out} (100%)
 create mode 100644 tests/qapi-schema/args-unknown.err
 rename tests/qapi-schema/{data-array-empty.exit => args-unknown.exit} (100%)
 rename tests/qapi-schema/{data-unknown.json => args-unknown.json} (100%)
 rename tests/qapi-schema/{data-array-empty.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 5271123..0d560c5 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 f713f14..cb7ed33 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-unknown.exit b/tests/qapi-schema/args-array-empty.exit
similarity index 100%
rename from tests/qapi-schema/data-unknown.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-unknown.out b/tests/qapi-schema/args-array-empty.out
similarity index 100%
rename from tests/qapi-schema/data-unknown.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 8b731bb..7834d11 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-member-unknown.exit b/tests/qapi-schema/args-array-unknown.exit
similarity index 100%
rename from tests/qapi-schema/data-member-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-member-unknown.out b/tests/qapi-schema/args-array-unknown.out
similarity index 100%
rename from tests/qapi-schema/data-member-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 0000000..dc1d250
--- /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-member-array-bad.exit b/tests/qapi-schema/args-int.exit
similarity index 100%
rename from tests/qapi-schema/data-member-array-bad.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-member-array.err b/tests/qapi-schema/args-int.out
similarity index 100%
rename from tests/qapi-schema/data-member-array.err
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 2c072d5..881b4d9 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-int.exit b/tests/qapi-schema/args-member-array-bad.exit
similarity index 100%
rename from tests/qapi-schema/data-int.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-int.out b/tests/qapi-schema/args-member-array.err
similarity index 100%
rename from tests/qapi-schema/data-int.out
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 0000000..f6f8282
--- /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-array-unknown.exit b/tests/qapi-schema/args-member-unknown.exit
similarity index 100%
rename from tests/qapi-schema/data-array-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-array-unknown.out b/tests/qapi-schema/args-member-unknown.out
similarity index 100%
rename from tests/qapi-schema/data-array-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 0000000..4d91ec8
--- /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-array-empty.exit b/tests/qapi-schema/args-unknown.exit
similarity index 100%
rename from tests/qapi-schema/data-array-empty.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-array-empty.out b/tests/qapi-schema/args-unknown.out
similarity index 100%
rename from tests/qapi-schema/data-array-empty.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 1a9b077..0000000
--- 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 ab905db..0000000
--- 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 5b07277..0000000
--- 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'
-- 
2.4.3

[Qemu-devel] [PULL 19/33] qapi-tests: New tests for union, alternate command arguments

[Markus Armbruster]

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 0d560c5..7315258 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 0000000..3086eae
--- /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 0000000..d00491f
--- /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 0000000..69e94d4
--- /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 0000000..e69de29
diff --git a/tests/qapi-schema/args-union.err b/tests/qapi-schema/args-union.err
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/args-union.exit b/tests/qapi-schema/args-union.exit
new file mode 100644
index 0000000..573541a
--- /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 0000000..db97ef6
--- /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 0000000..907080c
--- /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}]
+[]
-- 
2.4.3

[Qemu-devel] [PULL 20/33] qapi: Fix to reject union command and event arguments

[Markus Armbruster]

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 c2ac21c..c19c157 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 4879982..bbeae4d 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 e69de29..1d693d7 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 573541a..d00491f 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 db97ef6..7bdcbb7 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 907080c..e69de29 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}]
-[]
-- 
2.4.3

[Qemu-devel] [PULL 21/33] qapi: Command returning anonymous type doesn't work, outlaw

[Markus Armbruster]

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 +
 .../{nested-struct-returns.exit => returns-dict.exit}   |  0
 tests/qapi-schema/returns-dict.json                     |  2 ++
 .../{nested-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 c19c157..a253e27 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 bbeae4d..23c32fe 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 7315258..b8d445e 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 c90d408..9a62554 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 3d52d2b..efbe773 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 5238d07..0000000
--- 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 d2cd047..0000000
--- 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 0000000..eb2d0c4
--- /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 0000000..1cfef3e
--- /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
-- 
2.4.3

[Qemu-devel] [PULL 22/33] qapi-commands: Fix gen_err_check(e) for e and e != 'local_err'

[Markus Armbruster]

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 69029f5..3965ca8 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 = ""
-- 
2.4.3

[Qemu-devel] [PULL 23/33] qapi-commands: Inline gen_marshal_output_call()

[Markus Armbruster]

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 3965ca8..6de5229 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 = ""
 
-- 
2.4.3

[Qemu-devel] [PULL 24/33] qapi-commands: Don't feed output of mcgen() to mcgen() again

[Markus Armbruster]

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 6de5229..cfbd59c 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"
-- 
2.4.3

[Qemu-devel] [PULL 25/33] qapi-commands: Drop useless initialization

[Markus Armbruster]

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 a253e27..7cb852e 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 cfbd59c..8bf84a7 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)
-- 
2.4.3

[Qemu-devel] [PULL 26/33] qapi: Generated code cleanup

[Markus Armbruster]

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 7cb852e..b4d4a01 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 8bf84a7..890ce5d 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 88b0620..7f238df 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 8444f98..f2428f3 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 eec5f1f..3cd662b 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()
-- 
2.4.3

[Qemu-devel] [PULL 27/33] qapi: Drop one of two "simple union must not have base" checks

[Markus Armbruster]

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 23c32fe..197db77 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 fc8b79c..8b7a242 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
-- 
2.4.3

[Qemu-devel] [PULL 28/33] tests/qapi-schema: Cover two more syntax errors

[Markus Armbruster]

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 b8d445e..597ca90 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 0000000..f5c870b
--- /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 0000000..d00491f
--- /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 0000000..c5ba501
--- /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 0000000..e69de29
diff --git a/tests/qapi-schema/leading-comma-object.err b/tests/qapi-schema/leading-comma-object.err
new file mode 100644
index 0000000..f767b95
--- /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 0000000..d00491f
--- /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 0000000..c89023f
--- /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 0000000..e69de29
-- 
2.4.3

[Qemu-devel] [PULL 29/33] tests/qapi-schema: Cover non-string, non-dictionary members

[Markus Armbruster]

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 597ca90..b128e28 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 0000000..6d0fa05
--- /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 0000000..d00491f
--- /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 0000000..4a64135
--- /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 0000000..e69de29
diff --git a/tests/qapi-schema/struct-data-invalid.err b/tests/qapi-schema/struct-data-invalid.err
new file mode 100644
index 0000000..a40e23a
--- /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 0000000..d00491f
--- /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 0000000..b76de82
--- /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 0000000..e69de29
diff --git a/tests/qapi-schema/struct-member-invalid.err b/tests/qapi-schema/struct-member-invalid.err
new file mode 100644
index 0000000..9e4c7b8
--- /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 0000000..d00491f
--- /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 0000000..968e63c
--- /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 0000000..e69de29
-- 
2.4.3

[Qemu-devel] [PULL 30/33] qapi: Fix errors for non-string, non-dictionary members

[Markus Armbruster]

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 197db77..3656059 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 6d0fa05..fe1e949 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 4a64135..db09813 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 a40e23a..6644f4c 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 b76de82..9adbc3b 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 9e4c7b8..69a326d 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 968e63c..8f172f7 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 } }
-- 
2.4.3

[Qemu-devel] [PULL 31/33] qapi: Simplify error reporting for array types

[Markus Armbruster]

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 3656059..ac0e45e 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 7834d11..cd7a0f9 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 a41f019..f47c1ee 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'
-- 
2.4.3

[Qemu-devel] [PULL 32/33] docs/qapi-code-gen.txt: Fix QAPI schema examples

[Markus Armbruster]

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 b4d4a01..ff16df2 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' } }
 
 
-- 
2.4.3

[Qemu-devel] [PULL 33/33] qapi: Generators crash when --output-dir isn't given, fix

[Markus Armbruster]

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 ac0e45e..817d824 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:
-- 
2.4.3

Re: [Qemu-devel] [PULL 00/33] qapi: Another round of fixes and cleanups

[Peter Maydell]

On 4 September 2015 at 15:21, Markus Armbruster <armbru@redhat.com> wrote:
> The following changes since commit b041066421e8dcc7d080dfcfd83551c9c9f24ade:
>
>   Merge remote-tracking branch 'remotes/stefanha/tags/tracing-pull-request' into staging (2015-09-03 16:17:28 +0100)
>
> are available in the git repository at:
>
>   git://repo.or.cz/qemu/armbru.git tags/pull-qapi-2015-09-04
>
> for you to fetch changes up to c4f498fe8532cdacc609262b104322911108df54:
>
>   qapi: Generators crash when --output-dir isn't given, fix (2015-09-04 15:47:16 +0200)
>
> ----------------------------------------------------------------
> qapi: Another round of fixes and cleanups
>

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PULL 02/33] qapi: Clean up cgen() and mcgen()

[Laurent Desnogues]

Hello Markus,

I found some Python issue with this commit.

On Fri, Sep 4, 2015 at 4:21 PM, Markus Armbruster <armbru@redhat.com> wrote:
> 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          | 19 ++++++++++++-------
>  2 files changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
> index ca22acc..ce51408 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 06d7fc2..e656beb 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -943,16 +943,21 @@ 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'
> -
> -def mcgen(code, **kwds):
> -    raw = cgen('\n'.join(code.split('\n')[1:-1]), **kwds)
> +    raw = code % kwds
> +    if indent_level:
> +        indent = genindent(indent_level)
> +        raw = re.subn("^.", indent + r'\g<0>', raw, 0, re.MULTILINE)

subn with 5 parameters requires Python 2.7 which isn't part of CentOS/RHEL 6.

Thanks,

Laurent

> +        raw = raw[0]
>      return re.sub(re.escape(eatspace) + ' *', '', raw)
>
> +def mcgen(code, **kwds):
> +    if code[0] == '\n':
> +        code = code[1:]
> +    return cgen(code, **kwds)
> +
>  def basename(filename):
>      return filename.split("/")[-1]
>
> --
> 2.4.3
>
>

Re: [Qemu-devel] [PULL 02/33] qapi: Clean up cgen() and mcgen()

[Markus Armbruster]

Laurent Desnogues <laurent.desnogues@gmail.com> writes:

> Hello Markus,
>
> I found some Python issue with this commit.
>
> On Fri, Sep 4, 2015 at 4:21 PM, Markus Armbruster <armbru@redhat.com> wrote:
>> 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
[...]
>> index 06d7fc2..e656beb 100644
>> --- a/scripts/qapi.py
>> +++ b/scripts/qapi.py
>> @@ -943,16 +943,21 @@ 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'
>> -
>> -def mcgen(code, **kwds):
>> -    raw = cgen('\n'.join(code.split('\n')[1:-1]), **kwds)
>> +    raw = code % kwds
>> +    if indent_level:
>> +        indent = genindent(indent_level)
>> +        raw = re.subn("^.", indent + r'\g<0>', raw, 0, re.MULTILINE)
>
> subn with 5 parameters requires Python 2.7 which isn't part of CentOS/RHEL 6.

You're right.  I guess I need to fall back to re.compile().

[...]