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