From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35595) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZMgZK-0002vj-B3 for qemu-devel@nongnu.org; Tue, 04 Aug 2015 14:04:07 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZMgZF-0006zo-3E for qemu-devel@nongnu.org; Tue, 04 Aug 2015 14:04:06 -0400 Received: from mx1.redhat.com ([209.132.183.28]:59809) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZMgZE-0006zj-Rw for qemu-devel@nongnu.org; Tue, 04 Aug 2015 14:04:01 -0400 References: <1438679896-5077-1-git-send-email-armbru@redhat.com> <1438679896-5077-22-git-send-email-armbru@redhat.com> From: Eric Blake Message-ID: <55C0FE8A.9040000@redhat.com> Date: Tue, 4 Aug 2015 12:03:54 -0600 MIME-Version: 1.0 In-Reply-To: <1438679896-5077-22-git-send-email-armbru@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="dpuwXjkOLoCKRmi3AgEfNwa79JJlE5g6A" Subject: Re: [Qemu-devel] [PATCH 21/26] qapi: Command returning anonymous type doesn't work, outlaw List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster , qemu-devel@nongnu.org Cc: mdroth@linux.vnet.ibm.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --dpuwXjkOLoCKRmi3AgEfNwa79JJlE5g6A Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 08/04/2015 03:18 AM, Markus Armbruster wrote: > Reproducer: with >=20 > { 'command': 'user_def_cmd4', 'returns': { 'a': 'int' } } >=20 > added to qapi-schema-test.json, qapi-commands.py dies when it tries to > generate the command handler function >=20 > Traceback (most recent call last): > File "/work/armbru/qemu/scripts/qapi-commands.py", line 359, in <= module> > ret =3D generate_command_decl(cmd['command'], arglist, ret_type= ) + "\n" > File "/work/armbru/qemu/scripts/qapi-commands.py", line 29, in ge= nerate_command_decl > ret_type=3Dc_type(ret_type), name=3Dc_name(name), > File "/work/armbru/qemu/scripts/qapi.py", line 927, in c_type > assert isinstance(value, str) and value !=3D "" > AssertionError >=20 > because the return type doesn't exist. >=20 > 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. >=20 > Signed-off-by: Markus Armbruster > --- > 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 =3D> returns-dict.exit} | 0 > tests/qapi-schema/returns-dict.json | 2 ++ > .../{nested-struct-returns.out =3D> 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 =3D> returns-dict= =2Eexit} (100%) > create mode 100644 tests/qapi-schema/returns-dict.json > rename tests/qapi-schema/{nested-struct-returns.out =3D> returns-dict.= out} (100%) >=20 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 --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --dpuwXjkOLoCKRmi3AgEfNwa79JJlE5g6A Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJVwP6KAAoJEKeha0olJ0NqzCoIAKp8wDDXOQGFXe70qhSmcnTp d8nlcWPdj953lIrtXtoByNOMoYbkAz+e1geafIFO7k7RQ+5abm2TsbXgWJf4wUTY Kfd9zf4Bm7NBRbWrm6pTVefdvRZj8IIlWLUfjGr8x02Zgrhn6xRG0HtIMjfw3NoU Hv1D9iXmJNKdZRJnL7e0TYkQkxwg3/KoOMUYJA0S72M5zP2h2s63Qo8Sxidwx0Zj pq+GVhpTFkZ3ixoRAtSp1bT5moTtfZjZQmZFoXTfEiCFoIruTUVM/3z7iJq4eVHf DdYBiihxmbYxRd7zytGNFtLhBgTbXz1Q891cizKhfUxL1pGpqQhKs0LmOMR/HqE= =v/aT -----END PGP SIGNATURE----- --dpuwXjkOLoCKRmi3AgEfNwa79JJlE5g6A--