* [Qemu-devel] [PULL 01/33] qapi: Clarify docs on including the same file multiple times
2015-09-04 14:21 [Qemu-devel] [PULL 00/33] qapi: Another round of fixes and cleanups Markus Armbruster
@ 2015-09-04 14:21 ` Markus Armbruster
2015-09-04 14:21 ` [Qemu-devel] [PULL 02/33] qapi: Clean up cgen() and mcgen() Markus Armbruster
` (32 subsequent siblings)
33 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2015-09-04 14:21 UTC (permalink / raw)
To: qemu-devel
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
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL 02/33] qapi: Clean up cgen() and mcgen()
2015-09-04 14:21 [Qemu-devel] [PULL 00/33] qapi: Another round of fixes and cleanups Markus Armbruster
2015-09-04 14:21 ` [Qemu-devel] [PULL 01/33] qapi: Clarify docs on including the same file multiple times Markus Armbruster
@ 2015-09-04 14:21 ` Markus Armbruster
2015-09-07 12:42 ` Laurent Desnogues
2015-09-04 14:21 ` [Qemu-devel] [PULL 03/33] qapi: Simplify guardname() Markus Armbruster
` (31 subsequent siblings)
33 siblings, 1 reply; 37+ messages in thread
From: Markus Armbruster @ 2015-09-04 14:21 UTC (permalink / raw)
To: qemu-devel
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
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [Qemu-devel] [PULL 02/33] qapi: Clean up cgen() and mcgen()
2015-09-04 14:21 ` [Qemu-devel] [PULL 02/33] qapi: Clean up cgen() and mcgen() Markus Armbruster
@ 2015-09-07 12:42 ` Laurent Desnogues
2015-09-07 14:55 ` Markus Armbruster
0 siblings, 1 reply; 37+ messages in thread
From: Laurent Desnogues @ 2015-09-07 12:42 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel@nongnu.org
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
>
>
^ permalink raw reply [flat|nested] 37+ messages in thread* Re: [Qemu-devel] [PULL 02/33] qapi: Clean up cgen() and mcgen()
2015-09-07 12:42 ` Laurent Desnogues
@ 2015-09-07 14:55 ` Markus Armbruster
0 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2015-09-07 14:55 UTC (permalink / raw)
To: Laurent Desnogues; +Cc: qemu-devel@nongnu.org
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().
[...]
^ permalink raw reply [flat|nested] 37+ messages in thread
* [Qemu-devel] [PULL 03/33] qapi: Simplify guardname()
2015-09-04 14:21 [Qemu-devel] [PULL 00/33] qapi: Another round of fixes and cleanups Markus Armbruster
2015-09-04 14:21 ` [Qemu-devel] [PULL 01/33] qapi: Clarify docs on including the same file multiple times Markus Armbruster
2015-09-04 14:21 ` [Qemu-devel] [PULL 02/33] qapi: Clean up cgen() and mcgen() Markus Armbruster
@ 2015-09-04 14:21 ` Markus Armbruster
2015-09-04 14:21 ` [Qemu-devel] [PULL 04/33] qapi-event: Clean up how name of enum QAPIEvent is made Markus Armbruster
` (30 subsequent siblings)
33 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2015-09-04 14:21 UTC (permalink / raw)
To: qemu-devel
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
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL 04/33] qapi-event: Clean up how name of enum QAPIEvent is made
2015-09-04 14:21 [Qemu-devel] [PULL 00/33] qapi: Another round of fixes and cleanups Markus Armbruster
` (2 preceding siblings ...)
2015-09-04 14:21 ` [Qemu-devel] [PULL 03/33] qapi: Simplify guardname() Markus Armbruster
@ 2015-09-04 14:21 ` Markus Armbruster
2015-09-04 14:21 ` [Qemu-devel] [PULL 05/33] qapi: Reject -p arguments that break qapi-event.py Markus Armbruster
` (29 subsequent siblings)
33 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2015-09-04 14:21 UTC (permalink / raw)
To: qemu-devel
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
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL 05/33] qapi: Reject -p arguments that break qapi-event.py
2015-09-04 14:21 [Qemu-devel] [PULL 00/33] qapi: Another round of fixes and cleanups Markus Armbruster
` (3 preceding siblings ...)
2015-09-04 14:21 ` [Qemu-devel] [PULL 04/33] qapi-event: Clean up how name of enum QAPIEvent is made Markus Armbruster
@ 2015-09-04 14:21 ` Markus Armbruster
2015-09-04 14:21 ` [Qemu-devel] [PULL 06/33] qapi: Drop unused and useless parameters and variables Markus Armbruster
` (28 subsequent siblings)
33 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2015-09-04 14:21 UTC (permalink / raw)
To: qemu-devel
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
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL 06/33] qapi: Drop unused and useless parameters and variables
2015-09-04 14:21 [Qemu-devel] [PULL 00/33] qapi: Another round of fixes and cleanups Markus Armbruster
` (4 preceding siblings ...)
2015-09-04 14:21 ` [Qemu-devel] [PULL 05/33] qapi: Reject -p arguments that break qapi-event.py Markus Armbruster
@ 2015-09-04 14:21 ` Markus Armbruster
2015-09-04 14:21 ` [Qemu-devel] [PULL 07/33] qapi: Fix generated code when flat union has member 'kind' Markus Armbruster
` (27 subsequent siblings)
33 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2015-09-04 14:21 UTC (permalink / raw)
To: qemu-devel
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
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL 07/33] qapi: Fix generated code when flat union has member 'kind'
2015-09-04 14:21 [Qemu-devel] [PULL 00/33] qapi: Another round of fixes and cleanups Markus Armbruster
` (5 preceding siblings ...)
2015-09-04 14:21 ` [Qemu-devel] [PULL 06/33] qapi: Drop unused and useless parameters and variables Markus Armbruster
@ 2015-09-04 14:21 ` Markus Armbruster
2015-09-04 14:21 ` [Qemu-devel] [PULL 08/33] qapi: Generate a nicer struct for flat unions Markus Armbruster
` (26 subsequent siblings)
33 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2015-09-04 14:21 UTC (permalink / raw)
To: qemu-devel
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
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL 08/33] qapi: Generate a nicer struct for flat unions
2015-09-04 14:21 [Qemu-devel] [PULL 00/33] qapi: Another round of fixes and cleanups Markus Armbruster
` (6 preceding siblings ...)
2015-09-04 14:21 ` [Qemu-devel] [PULL 07/33] qapi: Fix generated code when flat union has member 'kind' Markus Armbruster
@ 2015-09-04 14:21 ` Markus Armbruster
2015-09-04 14:21 ` [Qemu-devel] [PULL 09/33] qapi-visit: Fix generated code when schema has forward refs Markus Armbruster
` (25 subsequent siblings)
33 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2015-09-04 14:21 UTC (permalink / raw)
To: qemu-devel
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
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL 09/33] qapi-visit: Fix generated code when schema has forward refs
2015-09-04 14:21 [Qemu-devel] [PULL 00/33] qapi: Another round of fixes and cleanups Markus Armbruster
` (7 preceding siblings ...)
2015-09-04 14:21 ` [Qemu-devel] [PULL 08/33] qapi: Generate a nicer struct for flat unions Markus Armbruster
@ 2015-09-04 14:21 ` Markus Armbruster
2015-09-04 14:21 ` [Qemu-devel] [PULL 10/33] qapi-visit: Replace list implicit_structs by set Markus Armbruster
` (24 subsequent siblings)
33 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2015-09-04 14:21 UTC (permalink / raw)
To: qemu-devel
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
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL 10/33] qapi-visit: Replace list implicit_structs by set
2015-09-04 14:21 [Qemu-devel] [PULL 00/33] qapi: Another round of fixes and cleanups Markus Armbruster
` (8 preceding siblings ...)
2015-09-04 14:21 ` [Qemu-devel] [PULL 09/33] qapi-visit: Fix generated code when schema has forward refs Markus Armbruster
@ 2015-09-04 14:21 ` Markus Armbruster
2015-09-04 14:21 ` [Qemu-devel] [PULL 11/33] qapi-visit: Fix two name arguments passed to visitors Markus Armbruster
` (23 subsequent siblings)
33 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2015-09-04 14:21 UTC (permalink / raw)
To: qemu-devel
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
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL 11/33] qapi-visit: Fix two name arguments passed to visitors
2015-09-04 14:21 [Qemu-devel] [PULL 00/33] qapi: Another round of fixes and cleanups Markus Armbruster
` (9 preceding siblings ...)
2015-09-04 14:21 ` [Qemu-devel] [PULL 10/33] qapi-visit: Replace list implicit_structs by set Markus Armbruster
@ 2015-09-04 14:21 ` Markus Armbruster
2015-09-04 14:21 ` [Qemu-devel] [PULL 12/33] tests/qapi-schema: Document alternate's enum lacks visit function Markus Armbruster
` (22 subsequent siblings)
33 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2015-09-04 14:21 UTC (permalink / raw)
To: qemu-devel
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
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL 12/33] tests/qapi-schema: Document alternate's enum lacks visit function
2015-09-04 14:21 [Qemu-devel] [PULL 00/33] qapi: Another round of fixes and cleanups Markus Armbruster
` (10 preceding siblings ...)
2015-09-04 14:21 ` [Qemu-devel] [PULL 11/33] qapi-visit: Fix two name arguments passed to visitors Markus Armbruster
@ 2015-09-04 14:21 ` Markus Armbruster
2015-09-04 14:21 ` [Qemu-devel] [PULL 13/33] tests/qapi-schema: Document events with base don't work Markus Armbruster
` (21 subsequent siblings)
33 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2015-09-04 14:21 UTC (permalink / raw)
To: qemu-devel
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
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL 13/33] tests/qapi-schema: Document events with base don't work
2015-09-04 14:21 [Qemu-devel] [PULL 00/33] qapi: Another round of fixes and cleanups Markus Armbruster
` (11 preceding siblings ...)
2015-09-04 14:21 ` [Qemu-devel] [PULL 12/33] tests/qapi-schema: Document alternate's enum lacks visit function Markus Armbruster
@ 2015-09-04 14:21 ` Markus Armbruster
2015-09-04 14:21 ` [Qemu-devel] [PULL 14/33] qapi: Document that input visitor semantics are prone to leaks Markus Armbruster
` (20 subsequent siblings)
33 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2015-09-04 14:21 UTC (permalink / raw)
To: qemu-devel
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
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL 14/33] qapi: Document that input visitor semantics are prone to leaks
2015-09-04 14:21 [Qemu-devel] [PULL 00/33] qapi: Another round of fixes and cleanups Markus Armbruster
` (12 preceding siblings ...)
2015-09-04 14:21 ` [Qemu-devel] [PULL 13/33] tests/qapi-schema: Document events with base don't work Markus Armbruster
@ 2015-09-04 14:21 ` Markus Armbruster
2015-09-04 14:21 ` [Qemu-devel] [PULL 15/33] qapi: Document shortcoming with union 'data' branch Markus Armbruster
` (19 subsequent siblings)
33 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2015-09-04 14:21 UTC (permalink / raw)
To: qemu-devel
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
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL 15/33] qapi: Document shortcoming with union 'data' branch
2015-09-04 14:21 [Qemu-devel] [PULL 00/33] qapi: Another round of fixes and cleanups Markus Armbruster
` (13 preceding siblings ...)
2015-09-04 14:21 ` [Qemu-devel] [PULL 14/33] qapi: Document that input visitor semantics are prone to leaks Markus Armbruster
@ 2015-09-04 14:21 ` Markus Armbruster
2015-09-04 14:21 ` [Qemu-devel] [PULL 16/33] qapi: Document flaws in checking of names Markus Armbruster
` (18 subsequent siblings)
33 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2015-09-04 14:21 UTC (permalink / raw)
To: qemu-devel
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
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL 16/33] qapi: Document flaws in checking of names
2015-09-04 14:21 [Qemu-devel] [PULL 00/33] qapi: Another round of fixes and cleanups Markus Armbruster
` (14 preceding siblings ...)
2015-09-04 14:21 ` [Qemu-devel] [PULL 15/33] qapi: Document shortcoming with union 'data' branch Markus Armbruster
@ 2015-09-04 14:21 ` Markus Armbruster
2015-09-04 14:21 ` [Qemu-devel] [PULL 17/33] tests/qapi-schema: Restore test case for flat union base bug Markus Armbruster
` (17 subsequent siblings)
33 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2015-09-04 14:21 UTC (permalink / raw)
To: qemu-devel
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
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL 17/33] tests/qapi-schema: Restore test case for flat union base bug
2015-09-04 14:21 [Qemu-devel] [PULL 00/33] qapi: Another round of fixes and cleanups Markus Armbruster
` (15 preceding siblings ...)
2015-09-04 14:21 ` [Qemu-devel] [PULL 16/33] qapi: Document flaws in checking of names Markus Armbruster
@ 2015-09-04 14:21 ` Markus Armbruster
2015-09-04 14:21 ` [Qemu-devel] [PULL 18/33] tests/qapi-schema: Rename tests from data- to args- Markus Armbruster
` (16 subsequent siblings)
33 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2015-09-04 14:21 UTC (permalink / raw)
To: qemu-devel
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
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL 18/33] tests/qapi-schema: Rename tests from data- to args-
2015-09-04 14:21 [Qemu-devel] [PULL 00/33] qapi: Another round of fixes and cleanups Markus Armbruster
` (16 preceding siblings ...)
2015-09-04 14:21 ` [Qemu-devel] [PULL 17/33] tests/qapi-schema: Restore test case for flat union base bug Markus Armbruster
@ 2015-09-04 14:21 ` Markus Armbruster
2015-09-04 14:21 ` [Qemu-devel] [PULL 19/33] qapi-tests: New tests for union, alternate command arguments Markus Armbruster
` (15 subsequent siblings)
33 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2015-09-04 14:21 UTC (permalink / raw)
To: qemu-devel
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
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL 19/33] qapi-tests: New tests for union, alternate command arguments
2015-09-04 14:21 [Qemu-devel] [PULL 00/33] qapi: Another round of fixes and cleanups Markus Armbruster
` (17 preceding siblings ...)
2015-09-04 14:21 ` [Qemu-devel] [PULL 18/33] tests/qapi-schema: Rename tests from data- to args- Markus Armbruster
@ 2015-09-04 14:21 ` Markus Armbruster
2015-09-04 14:21 ` [Qemu-devel] [PULL 20/33] qapi: Fix to reject union command and event arguments Markus Armbruster
` (14 subsequent siblings)
33 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2015-09-04 14:21 UTC (permalink / raw)
To: qemu-devel
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
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL 20/33] qapi: Fix to reject union command and event arguments
2015-09-04 14:21 [Qemu-devel] [PULL 00/33] qapi: Another round of fixes and cleanups Markus Armbruster
` (18 preceding siblings ...)
2015-09-04 14:21 ` [Qemu-devel] [PULL 19/33] qapi-tests: New tests for union, alternate command arguments Markus Armbruster
@ 2015-09-04 14:21 ` Markus Armbruster
2015-09-04 14:21 ` [Qemu-devel] [PULL 21/33] qapi: Command returning anonymous type doesn't work, outlaw Markus Armbruster
` (13 subsequent siblings)
33 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2015-09-04 14:21 UTC (permalink / raw)
To: qemu-devel
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
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL 21/33] qapi: Command returning anonymous type doesn't work, outlaw
2015-09-04 14:21 [Qemu-devel] [PULL 00/33] qapi: Another round of fixes and cleanups Markus Armbruster
` (19 preceding siblings ...)
2015-09-04 14:21 ` [Qemu-devel] [PULL 20/33] qapi: Fix to reject union command and event arguments Markus Armbruster
@ 2015-09-04 14:21 ` Markus Armbruster
2015-09-04 14:21 ` [Qemu-devel] [PULL 22/33] qapi-commands: Fix gen_err_check(e) for e and e != 'local_err' Markus Armbruster
` (12 subsequent siblings)
33 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2015-09-04 14:21 UTC (permalink / raw)
To: qemu-devel
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
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL 22/33] qapi-commands: Fix gen_err_check(e) for e and e != 'local_err'
2015-09-04 14:21 [Qemu-devel] [PULL 00/33] qapi: Another round of fixes and cleanups Markus Armbruster
` (20 preceding siblings ...)
2015-09-04 14:21 ` [Qemu-devel] [PULL 21/33] qapi: Command returning anonymous type doesn't work, outlaw Markus Armbruster
@ 2015-09-04 14:21 ` Markus Armbruster
2015-09-04 14:21 ` [Qemu-devel] [PULL 23/33] qapi-commands: Inline gen_marshal_output_call() Markus Armbruster
` (11 subsequent siblings)
33 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2015-09-04 14:21 UTC (permalink / raw)
To: qemu-devel
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
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL 23/33] qapi-commands: Inline gen_marshal_output_call()
2015-09-04 14:21 [Qemu-devel] [PULL 00/33] qapi: Another round of fixes and cleanups Markus Armbruster
` (21 preceding siblings ...)
2015-09-04 14:21 ` [Qemu-devel] [PULL 22/33] qapi-commands: Fix gen_err_check(e) for e and e != 'local_err' Markus Armbruster
@ 2015-09-04 14:21 ` Markus Armbruster
2015-09-04 14:21 ` [Qemu-devel] [PULL 24/33] qapi-commands: Don't feed output of mcgen() to mcgen() again Markus Armbruster
` (10 subsequent siblings)
33 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2015-09-04 14:21 UTC (permalink / raw)
To: qemu-devel
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
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL 24/33] qapi-commands: Don't feed output of mcgen() to mcgen() again
2015-09-04 14:21 [Qemu-devel] [PULL 00/33] qapi: Another round of fixes and cleanups Markus Armbruster
` (22 preceding siblings ...)
2015-09-04 14:21 ` [Qemu-devel] [PULL 23/33] qapi-commands: Inline gen_marshal_output_call() Markus Armbruster
@ 2015-09-04 14:21 ` Markus Armbruster
2015-09-04 14:21 ` [Qemu-devel] [PULL 25/33] qapi-commands: Drop useless initialization Markus Armbruster
` (9 subsequent siblings)
33 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2015-09-04 14:21 UTC (permalink / raw)
To: qemu-devel
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
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL 25/33] qapi-commands: Drop useless initialization
2015-09-04 14:21 [Qemu-devel] [PULL 00/33] qapi: Another round of fixes and cleanups Markus Armbruster
` (23 preceding siblings ...)
2015-09-04 14:21 ` [Qemu-devel] [PULL 24/33] qapi-commands: Don't feed output of mcgen() to mcgen() again Markus Armbruster
@ 2015-09-04 14:21 ` Markus Armbruster
2015-09-04 14:21 ` [Qemu-devel] [PULL 26/33] qapi: Generated code cleanup Markus Armbruster
` (8 subsequent siblings)
33 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2015-09-04 14:21 UTC (permalink / raw)
To: qemu-devel
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
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL 26/33] qapi: Generated code cleanup
2015-09-04 14:21 [Qemu-devel] [PULL 00/33] qapi: Another round of fixes and cleanups Markus Armbruster
` (24 preceding siblings ...)
2015-09-04 14:21 ` [Qemu-devel] [PULL 25/33] qapi-commands: Drop useless initialization Markus Armbruster
@ 2015-09-04 14:21 ` Markus Armbruster
2015-09-04 14:21 ` [Qemu-devel] [PULL 27/33] qapi: Drop one of two "simple union must not have base" checks Markus Armbruster
` (7 subsequent siblings)
33 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2015-09-04 14:21 UTC (permalink / raw)
To: qemu-devel
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
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL 27/33] qapi: Drop one of two "simple union must not have base" checks
2015-09-04 14:21 [Qemu-devel] [PULL 00/33] qapi: Another round of fixes and cleanups Markus Armbruster
` (25 preceding siblings ...)
2015-09-04 14:21 ` [Qemu-devel] [PULL 26/33] qapi: Generated code cleanup Markus Armbruster
@ 2015-09-04 14:21 ` Markus Armbruster
2015-09-04 14:21 ` [Qemu-devel] [PULL 28/33] tests/qapi-schema: Cover two more syntax errors Markus Armbruster
` (6 subsequent siblings)
33 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2015-09-04 14:21 UTC (permalink / raw)
To: qemu-devel
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
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL 28/33] tests/qapi-schema: Cover two more syntax errors
2015-09-04 14:21 [Qemu-devel] [PULL 00/33] qapi: Another round of fixes and cleanups Markus Armbruster
` (26 preceding siblings ...)
2015-09-04 14:21 ` [Qemu-devel] [PULL 27/33] qapi: Drop one of two "simple union must not have base" checks Markus Armbruster
@ 2015-09-04 14:21 ` Markus Armbruster
2015-09-04 14:21 ` [Qemu-devel] [PULL 29/33] tests/qapi-schema: Cover non-string, non-dictionary members Markus Armbruster
` (5 subsequent siblings)
33 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2015-09-04 14:21 UTC (permalink / raw)
To: qemu-devel
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
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL 29/33] tests/qapi-schema: Cover non-string, non-dictionary members
2015-09-04 14:21 [Qemu-devel] [PULL 00/33] qapi: Another round of fixes and cleanups Markus Armbruster
` (27 preceding siblings ...)
2015-09-04 14:21 ` [Qemu-devel] [PULL 28/33] tests/qapi-schema: Cover two more syntax errors Markus Armbruster
@ 2015-09-04 14:21 ` Markus Armbruster
2015-09-04 14:21 ` [Qemu-devel] [PULL 30/33] qapi: Fix errors for " Markus Armbruster
` (4 subsequent siblings)
33 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2015-09-04 14:21 UTC (permalink / raw)
To: qemu-devel
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
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL 30/33] qapi: Fix errors for non-string, non-dictionary members
2015-09-04 14:21 [Qemu-devel] [PULL 00/33] qapi: Another round of fixes and cleanups Markus Armbruster
` (28 preceding siblings ...)
2015-09-04 14:21 ` [Qemu-devel] [PULL 29/33] tests/qapi-schema: Cover non-string, non-dictionary members Markus Armbruster
@ 2015-09-04 14:21 ` Markus Armbruster
2015-09-04 14:21 ` [Qemu-devel] [PULL 31/33] qapi: Simplify error reporting for array types Markus Armbruster
` (3 subsequent siblings)
33 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2015-09-04 14:21 UTC (permalink / raw)
To: qemu-devel
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
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL 31/33] qapi: Simplify error reporting for array types
2015-09-04 14:21 [Qemu-devel] [PULL 00/33] qapi: Another round of fixes and cleanups Markus Armbruster
` (29 preceding siblings ...)
2015-09-04 14:21 ` [Qemu-devel] [PULL 30/33] qapi: Fix errors for " Markus Armbruster
@ 2015-09-04 14:21 ` Markus Armbruster
2015-09-04 14:21 ` [Qemu-devel] [PULL 32/33] docs/qapi-code-gen.txt: Fix QAPI schema examples Markus Armbruster
` (2 subsequent siblings)
33 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2015-09-04 14:21 UTC (permalink / raw)
To: qemu-devel
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
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL 32/33] docs/qapi-code-gen.txt: Fix QAPI schema examples
2015-09-04 14:21 [Qemu-devel] [PULL 00/33] qapi: Another round of fixes and cleanups Markus Armbruster
` (30 preceding siblings ...)
2015-09-04 14:21 ` [Qemu-devel] [PULL 31/33] qapi: Simplify error reporting for array types Markus Armbruster
@ 2015-09-04 14:21 ` Markus Armbruster
2015-09-04 14:21 ` [Qemu-devel] [PULL 33/33] qapi: Generators crash when --output-dir isn't given, fix Markus Armbruster
2015-09-04 16:37 ` [Qemu-devel] [PULL 00/33] qapi: Another round of fixes and cleanups Peter Maydell
33 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2015-09-04 14:21 UTC (permalink / raw)
To: qemu-devel
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
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL 33/33] qapi: Generators crash when --output-dir isn't given, fix
2015-09-04 14:21 [Qemu-devel] [PULL 00/33] qapi: Another round of fixes and cleanups Markus Armbruster
` (31 preceding siblings ...)
2015-09-04 14:21 ` [Qemu-devel] [PULL 32/33] docs/qapi-code-gen.txt: Fix QAPI schema examples Markus Armbruster
@ 2015-09-04 14:21 ` Markus Armbruster
2015-09-04 16:37 ` [Qemu-devel] [PULL 00/33] qapi: Another round of fixes and cleanups Peter Maydell
33 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2015-09-04 14:21 UTC (permalink / raw)
To: qemu-devel
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
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [Qemu-devel] [PULL 00/33] qapi: Another round of fixes and cleanups
2015-09-04 14:21 [Qemu-devel] [PULL 00/33] qapi: Another round of fixes and cleanups Markus Armbruster
` (32 preceding siblings ...)
2015-09-04 14:21 ` [Qemu-devel] [PULL 33/33] qapi: Generators crash when --output-dir isn't given, fix Markus Armbruster
@ 2015-09-04 16:37 ` Peter Maydell
33 siblings, 0 replies; 37+ messages in thread
From: Peter Maydell @ 2015-09-04 16:37 UTC (permalink / raw)
To: Markus Armbruster; +Cc: QEMU Developers
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
^ permalink raw reply [flat|nested] 37+ messages in thread