On 02/27/2014 04:09 AM, Wenchao Xia wrote: > From: Wenchao Xia <xiaw...@linux.vnet.ibm.com> > > It is bad that same key was specified twice, especially when a union have
s/have/has/ > two branches with same condition. This patch can prevent it. > > Signed-off-by: Wenchao Xia <xiaw...@linux.vnet.ibm.com> > Signed-off-by: Wenchao Xia <wenchaoq...@gmail.com> 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 > > +++ 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 <ebl...@redhat.com> -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature