From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59504) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WJ3V5-0007jK-Mp for qemu-devel@nongnu.org; Thu, 27 Feb 2014 11:08:00 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WJ3V1-0004UU-2t for qemu-devel@nongnu.org; Thu, 27 Feb 2014 11:07:55 -0500 Received: from mx1.redhat.com ([209.132.183.28]:65529) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WJ3V0-0004UE-QK for qemu-devel@nongnu.org; Thu, 27 Feb 2014 11:07:50 -0500 Message-ID: <530F5C93.1000308@redhat.com> Date: Thu, 27 Feb 2014 08:41:07 -0700 From: Eric Blake MIME-Version: 1.0 References: <1393499376-4374-1-git-send-email-wenchaoqemu@gmail.com> <1393499376-4374-3-git-send-email-wenchaoqemu@gmail.com> In-Reply-To: <1393499376-4374-3-git-send-email-wenchaoqemu@gmail.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="0HRAU0AjcIGxkBTHEBEnWCp9N82j7gdpW" Subject: Re: [Qemu-devel] [PATCH V8 02/10] qapi script: add check for duplicated key List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Wenchao Xia , qemu-devel@nongnu.org Cc: mdroth@linux.vnet.ibm.com, kwolf@redhat.com, Wenchao Xia , armbru@redhat.com, lcapitulino@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --0HRAU0AjcIGxkBTHEBEnWCp9N82j7gdpW Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 02/27/2014 04:09 AM, Wenchao Xia wrote: > From: Wenchao Xia >=20 > It is bad that same key was specified twice, especially when a union ha= ve s/have/has/ > two branches with same condition. This patch can prevent it. >=20 > Signed-off-by: Wenchao Xia > Signed-off-by: Wenchao Xia Again, the double S-o-B is odd. > --- > scripts/qapi.py | 2 ++ > tests/Makefile | 3 ++- > tests/qapi-schema/duplicate-key.err | 1 + > tests/qapi-schema/duplicate-key.exit | 1 + > tests/qapi-schema/duplicate-key.json | 2 ++ > 5 files changed, 8 insertions(+), 1 deletions(-) > create mode 100644 tests/qapi-schema/duplicate-key.err > create mode 100644 tests/qapi-schema/duplicate-key.exit > create mode 100644 tests/qapi-schema/duplicate-key.json > create mode 100644 tests/qapi-schema/duplicate-key.out >=20 > +++ b/tests/qapi-schema/duplicate-key.json > @@ -0,0 +1,2 @@ > +{ 'key': 'value', > + 'key': 'value' } This tests a duplicate key in a dictionary. Since unions use 'data':{dictionary}, that means you caught duplicate branches within a union. Based on your test, your patch also has the nice effect of catching duplicates unrelated to unions! This test is of a top-level struct; a bit more realistic might be: { 'command': 'foo', 'command': 'foo', 'data': {} } Meanwhile, should we also test for duplicates in non-dict locations, such as: { 'enum': 'Foo', 'data': [ 'value', 'value' ] } or even tests of duplicate top-level items, such as: { 'type': 'Dup', 'data': { 'field':'str' } } { 'type': 'Dup', 'data': { 'field':'str' } } But I'm okay with that as a followup, in the interest of getting this series in. Reviewed-by: Eric Blake --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --0HRAU0AjcIGxkBTHEBEnWCp9N82j7gdpW 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/ iQEcBAEBCAAGBQJTD1yTAAoJEKeha0olJ0NqmpYH/RbtXiWSEMPk1hr++v84UHNJ NaNXJ6ATRjov1RtCnpMF5WZClRlLibfk1zWFuaaSQ0jiUqZANAMEEvgBKbBcWGNQ xb1kZoCuhbwJUUs1oG8CRZYYMQiuyNInJ3jVzqHNtsaup+4ZF44uX2kJ8lSjjeiS QdEJMR/TlRvPlSGZW7VunmYsqhNYvKPxmwdN/XQoqQbWcYyg/DTw0VtVcpZULkVZ WGUOCWqo0LqHzTsXhn9zPormzJeSMkxF7/fKcUH8gmTM9z0XjXEYfBzFsfOpLhk9 1YQMWDyz0g5z73YphK9sVgR+vImwMJi9U199ckcoz8I19HrXzsHnc4eZflz5lJU= =YVRi -----END PGP SIGNATURE----- --0HRAU0AjcIGxkBTHEBEnWCp9N82j7gdpW--