Skip to content

Commit

Permalink
qapi: Command returning anonymous type doesn't work, outlaw
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Markus Armbruster committed Sep 4, 2015
1 parent 315932b commit 9b090d4
Show file tree
Hide file tree
Showing 11 changed files with 16 additions and 20 deletions.
17 changes: 8 additions & 9 deletions docs/qapi-code-gen.txt
Expand Up @@ -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,
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion scripts/qapi.py
Expand Up @@ -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)

Expand Down
4 changes: 2 additions & 2 deletions tests/Makefile
Expand Up @@ -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 \
Expand Down
3 changes: 1 addition & 2 deletions 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' } }
3 changes: 1 addition & 2 deletions 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' } }
1 change: 0 additions & 1 deletion tests/qapi-schema/nested-struct-returns.err

This file was deleted.

3 changes: 0 additions & 3 deletions tests/qapi-schema/nested-struct-returns.json

This file was deleted.

1 change: 1 addition & 0 deletions 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
File renamed without changes.
2 changes: 2 additions & 0 deletions tests/qapi-schema/returns-dict.json
@@ -0,0 +1,2 @@
# we reject inline struct return type
{ 'command': 'oops', 'returns': { 'a': 'str' } }
File renamed without changes.

0 comments on commit 9b090d4

Please sign in to comment.