From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44126) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YbaTR-0004SN-1P for qemu-devel@nongnu.org; Fri, 27 Mar 2015 16:03:22 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YbaTM-0003Jz-VM for qemu-devel@nongnu.org; Fri, 27 Mar 2015 16:03:20 -0400 Received: from mx1.redhat.com ([209.132.183.28]:40451) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YbaTM-0003Ji-OU for qemu-devel@nongnu.org; Fri, 27 Mar 2015 16:03:16 -0400 Message-ID: <5515B781.4060201@redhat.com> Date: Fri, 27 Mar 2015 14:03:13 -0600 From: Eric Blake MIME-Version: 1.0 References: <1427227433-5030-1-git-send-email-eblake@redhat.com> <1427227433-5030-21-git-send-email-eblake@redhat.com> <87r3sau9vl.fsf@blackfin.pond.sub.org> In-Reply-To: <87r3sau9vl.fsf@blackfin.pond.sub.org> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="hmffuXlVmgFmGddo7hKBjpKXdcwLu6M9l" Subject: Re: [Qemu-devel] [PATCH v5 20/28] qapi: More rigourous checking of types List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: kwolf@redhat.com, lcapitulino@redhat.com, famz@redhat.com, qemu-devel@nongnu.org, wenchaoqemu@gmail.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --hmffuXlVmgFmGddo7hKBjpKXdcwLu6M9l Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 03/27/2015 02:23 AM, Markus Armbruster wrote: > Eric Blake writes: >=20 >> Now that we know every expression is valid with regards to >> its keys, we can add further tests that those keys refer to >> valid types. With this patch, all uses of a type (the 'data': >> of command, type, union, alternate, and event; the 'returns': >> of command; the 'base': of type and union) must resolve to an >> appropriate subset of metatypes declared by the current qapi >> parse; this includes recursing into each member of a data >> dictionary. Dealing with '**' and nested anonymous structs >> will be done in later patches. >> >> Update the testsuite to match improved output. >> >> Signed-off-by: Eric Blake >> --- > [...] >> diff --git a/scripts/qapi.py b/scripts/qapi.py >> index 6ed6a34..c42683b 100644 >> --- a/scripts/qapi.py >> +++ b/scripts/qapi.py >> @@ -276,6 +276,63 @@ def discriminator_find_enum_define(expr): >> >> return find_enum(discriminator_type) >> >> +def check_type(expr_info, source, data, allow_array =3D False, >> + allowed_metas =3D [], allow_dict =3D True): >=20 > I'd allow_dict =3D False here, because I like defaulting to false. Mat= ter > of taste. Not too hard. >=20 > I'd name the third parameter def rather than data. Matter of taste > again. >=20 Oh, definitely a better name. When I first prototyped the function, I was only crawling the 'data' member, but since I now also use it to crawl 'returns' and 'base' it does make a difference. >> + for (key, value) in data.items(): >> + check_type(expr_info, "Member '%s' of %s" % (key, source), va= lue, >=20 > This can produce messages like >=20 > Member 'inner' of Member 'outer' of ... >=20 > I figure the problem will go away when you ditch nested structs. Not > worth worrying about it then. Yep, it disappears quite nicely. (I was torn on whether to make the message start with lower-case, to avoid the mid-sentence Cap, but the prevailing practice in this file before my patches was to start all QAPIExprError with a capital) >> >> +def check_struct(expr, expr_info): >> + name =3D expr['type'] >> + members =3D expr['data'] >> + >> + check_type(expr_info, "'data' for type '%s'" % name, members) >=20 > This one gave me pause, until I realized that allowed_metas=3D[], > allow_dict=3DTrue accepts exactly dictionary members, which is what we > want. And your idea of having allow_dict default to False in the prototype to make it explicit here might make it easier to understand (where the default allows nothing, and then callers add the pieces that they want). Indeed, just typing this email, that sounds nicer than the current case of defaults cater to the majority, but force some callers to have to explicitly opt out. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --hmffuXlVmgFmGddo7hKBjpKXdcwLu6M9l Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJVFbeBAAoJEKeha0olJ0NqBKQH/1EjpSuhwA7rtRlS4xs6+oBt GbQBh4h/OhqscFmH7R3TUOR3jEJFQHElz89J9sSVyR3mutVmYWss6/CT/kMO+dAP 2lj3QMocjf67aRkP3owjre2ORrjQGmveibOMAg5kW27saJLW7J3n7uKnu24uILnI ISXFZ9VDfRnO6S+1AOq/bTpympCXrVt4RqCW/hLbUecIrA5KXr50XwwVtD3eqzTl 4tYHN3/5Zi0x0bIKbCi6HjDdDgX/yfnXMJ+yf0JqVCCAunU7zRBLpZSpRsm8TVrh UoM/CT365z/dR8XBOPU9r+Or96LKViOB4TVbRDlSc0S6NXesSzy/wDa3kWqxXtM= =M+o2 -----END PGP SIGNATURE----- --hmffuXlVmgFmGddo7hKBjpKXdcwLu6M9l--