[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 21/26] qapi: Command returning anonymous type do
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH 21/26] qapi: Command returning anonymous type doesn't work, outlaw |
Date: |
Tue, 4 Aug 2015 12:03:54 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0 |
On 08/04/2015 03:18 AM, Markus Armbruster wrote:
> 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.
Might be worth allowing someday, but that would imply that we can come
up with a sane naming scheme for anonymous structs in the qapi schema
that won't risk collisions with explicit types. Shame on me for not
thinking to test this in my earlier testsuite additions, but I
definitely agree with your solution of outlawing it for now.
>
> Signed-off-by: Markus Armbruster <address@hidden>
> ---
> docs/qapi-code-gen.txt | 17
> ++++++++---------
> scripts/qapi.py | 2 +-
> tests/Makefile | 4 ++--
> 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
> 9 files changed, 14 insertions(+), 16 deletions(-)
Once again, git rename detection didn't accurately capture what you did :)
> 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%)
>
git grep "'returns'.*{"
found a couple more culprits (tests that fail elsewhere prior to warning
about this, but where an anonymous return does not add to the negative
test). Please squash this in:
diff --git i/tests/qapi-schema/command-int.json
w/tests/qapi-schema/command-int.json
index c90d408..40a6ae3 100644
--- i/tests/qapi-schema/command-int.json
+++ w/tests/qapi-schema/command-int.json
@@ -1,3 +1,4 @@
# we reject collisions between commands and types
{ 'command': 'int', 'data': { 'character': 'str' },
- 'returns': { 'value': 'int' } }
+ 'returns': 'Foo' }
+{ 'struct': 'Foo', 'data': { 'value': 'int' } }
diff --git i/tests/qapi-schema/nested-struct-data.json
w/tests/qapi-schema/nested-struct-data.json
index 3d52d2b..efbe773 100644
--- i/tests/qapi-schema/nested-struct-data.json
+++ w/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' } }
at which point you may add:
Reviewed-by: Eric Blake <address@hidden>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [PATCH 23/26] qapi-commands: Inline gen_marshal_output_call(), (continued)
- [Qemu-devel] [PATCH 23/26] qapi-commands: Inline gen_marshal_output_call(), Markus Armbruster, 2015/08/04
- [Qemu-devel] [PATCH 11/26] qapi-visit: Fix two name arguments passed to visitors, Markus Armbruster, 2015/08/04
- [Qemu-devel] [PATCH 12/26] tests/qapi-schema: Document alternate's enum lacks visit function, Markus Armbruster, 2015/08/04
- [Qemu-devel] [PATCH 22/26] qapi-commands: Fix gen_err_check(e) for e and e != 'local_err', Markus Armbruster, 2015/08/04
- [Qemu-devel] [PATCH 26/26] qapi: Generated code cleanup, Markus Armbruster, 2015/08/04
- [Qemu-devel] [PATCH 18/26] tests/qapi-schema: Rename tests from data- to args-, Markus Armbruster, 2015/08/04
- [Qemu-devel] [PATCH 24/26] qapi-commands: Don't feed output of mcgen() to mcgen() again, Markus Armbruster, 2015/08/04
- [Qemu-devel] [PATCH 21/26] qapi: Command returning anonymous type doesn't work, outlaw, Markus Armbruster, 2015/08/04
- Re: [Qemu-devel] [PATCH 21/26] qapi: Command returning anonymous type doesn't work, outlaw,
Eric Blake <=