All of lore.kernel.org
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Cc: kwolf@redhat.com, mdroth@linux.vnet.ibm.com,
	qemu-devel@nongnu.org, lcapitulino@redhat.com
Subject: Re: [Qemu-devel] [PATCH V6 02/10] qapi script: add check for duplicated key
Date: Mon, 17 Feb 2014 09:28:30 +0100	[thread overview]
Message-ID: <878uta2le9.fsf@blackfin.pond.sub.org> (raw)
In-Reply-To: <52FD7667.3070309@linux.vnet.ibm.com> (Wenchao Xia's message of "Fri, 14 Feb 2014 09:50:31 +0800")

Wenchao Xia <xiawenc@linux.vnet.ibm.com> writes:

> 于 2014/2/13 23:14, Markus Armbruster 写道:
>> Wenchao Xia <xiawenc@linux.vnet.ibm.com> writes:
>> 
>>> It is bad that same key was specified twice, especially when a union have
>>> two branches with same condition. This patch can prevent it.
>>>
>>> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
>>> Reviewed-by: Eric Blake <eblake@redhat.com>
>>> ---
>>>   scripts/qapi.py |    2 ++
>>>   1 files changed, 2 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/scripts/qapi.py b/scripts/qapi.py
>>> index aec6bbb..cf34768 100644
>>> --- a/scripts/qapi.py
>>> +++ b/scripts/qapi.py
>>> @@ -116,6 +116,8 @@ class QAPISchema:
>>>               if self.tok != ':':
>>>                   raise QAPISchemaError(self, 'Expected ":"')
>>>               self.accept()
>>> +            if key in expr:
>>> +                raise QAPISchemaError(self, 'Duplicated key "%s"' % key)
>>>               expr[key] = self.get_expr(True)
>>>               if self.tok == '}':
>>>                   self.accept()
>> 
>> All errors should have a test in tests/qapi-schema/.  I can try to add
>> tests for you when I rebase your 09/10.
>> 
>   I considered error path test before but didn't find a good place to
> go. It would be great if you can add one.

Here's the test for your commit, feel free to squash it into yours.


>From ce842b83cd999ee76e4221e24313a5c447e40bac Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Fri, 14 Feb 2014 13:15:46 +0100
Subject: [PATCH] qapi: Polish error message for duplicate key, and add test

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/qapi.py                      | 2 +-
 tests/Makefile                       | 4 ++--
 tests/qapi-schema/duplicate-key.err  | 1 +
 tests/qapi-schema/duplicate-key.exit | 1 +
 tests/qapi-schema/duplicate-key.json | 2 ++
 tests/qapi-schema/duplicate-key.out  | 0
 6 files changed, 7 insertions(+), 3 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

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 0663c2e..3732fe1 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -117,7 +117,7 @@ class QAPISchema:
                 raise QAPISchemaError(self, 'Expected ":"')
             self.accept()
             if key in expr:
-                raise QAPISchemaError(self, 'Duplicated key "%s"' % key)
+                raise QAPISchemaError(self, 'Duplicate key "%s"' % key)
             expr[key] = self.get_expr(True)
             if self.tok == '}':
                 self.accept()
diff --git a/tests/Makefile b/tests/Makefile
index fd36eee..e3ddbcc 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -119,8 +119,8 @@ check-qtest-xtensa-y += tests/qom-test$(EXESUF)
 check-qtest-xtensaeb-y = $(check-qtest-xtensa-y)
 
 check-qapi-schema-y := $(addprefix tests/qapi-schema/, \
-        comments.json empty.json funny-char.json indented-expr.json \
-        missing-colon.json missing-comma-list.json \
+        comments.json duplicate-key.json empty.json funny-char.json \
+        indented-expr.json missing-colon.json missing-comma-list.json \
         missing-comma-object.json non-objects.json \
         qapi-schema-test.json quoted-structural-chars.json \
         trailing-comma-list.json trailing-comma-object.json \
diff --git a/tests/qapi-schema/duplicate-key.err b/tests/qapi-schema/duplicate-key.err
new file mode 100644
index 0000000..0801c6a
--- /dev/null
+++ b/tests/qapi-schema/duplicate-key.err
@@ -0,0 +1 @@
+<stdin>:2:10: Duplicate key "key"
diff --git a/tests/qapi-schema/duplicate-key.exit b/tests/qapi-schema/duplicate-key.exit
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/tests/qapi-schema/duplicate-key.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/duplicate-key.json b/tests/qapi-schema/duplicate-key.json
new file mode 100644
index 0000000..1b55d88
--- /dev/null
+++ b/tests/qapi-schema/duplicate-key.json
@@ -0,0 +1,2 @@
+{ 'key': 'value',
+  'key': 'value' }
diff --git a/tests/qapi-schema/duplicate-key.out b/tests/qapi-schema/duplicate-key.out
new file mode 100644
index 0000000..e69de29
-- 
1.8.1.4

  reply	other threads:[~2014-02-17  8:28 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-02-10 21:48 [Qemu-devel] [PATCH V6 00/10] qapi script: support enum as discriminator and better enum name Wenchao Xia
2014-02-10 21:48 ` [Qemu-devel] [PATCH V6 01/10] qapi script: remember enum values Wenchao Xia
2014-02-10 21:48 ` [Qemu-devel] [PATCH V6 02/10] qapi script: add check for duplicated key Wenchao Xia
2014-02-13 15:14   ` Markus Armbruster
2014-02-14  1:50     ` Wenchao Xia
2014-02-17  8:28       ` Markus Armbruster [this message]
2014-02-10 21:48 ` [Qemu-devel] [PATCH V6 03/10] qapi script: check correctness of discriminator values in union Wenchao Xia
2014-02-13 15:14   ` Markus Armbruster
2014-02-14  2:43     ` Wenchao Xia
2014-02-14  9:23       ` Markus Armbruster
2014-02-17  1:50         ` Wenchao Xia
2014-02-17  8:11           ` Markus Armbruster
2014-02-17 13:59           ` Luiz Capitulino
2014-02-10 21:48 ` [Qemu-devel] [PATCH V6 04/10] qapi script: code move for generate_enum_name() Wenchao Xia
2014-02-10 21:48 ` [Qemu-devel] [PATCH V6 05/10] qapi script: use same function to generate enum string Wenchao Xia
2014-02-10 21:48 ` [Qemu-devel] [PATCH V6 06/10] qapi script: support pre-defined enum type as discriminator in union Wenchao Xia
2014-02-10 21:48 ` [Qemu-devel] [PATCH V6 07/10] qapi: convert BlockdevOptions to use enum discriminator Wenchao Xia
2014-02-10 21:48 ` [Qemu-devel] [PATCH V6 08/10] qapi script: do not allow string discriminator Wenchao Xia
2014-02-13 15:18   ` Markus Armbruster
2014-02-10 21:48 ` [Qemu-devel] [PATCH V6 09/10] tests: add cases for inherited struct and union with discriminator Wenchao Xia
2014-02-13 14:53   ` Markus Armbruster
2014-02-13 15:11     ` Luiz Capitulino
2014-02-14  2:47       ` Wenchao Xia
2014-02-10 21:48 ` [Qemu-devel] [PATCH V6 10/10] qapi script: do not add "_" for every capitalized char in enum Wenchao Xia
2014-02-11 13:17 ` [Qemu-devel] [PATCH V6 00/10] qapi script: support enum as discriminator and better enum name Eric Blake
2014-02-11 19:13 ` Luiz Capitulino
2014-02-13 15:22   ` Luiz Capitulino
2014-02-13 15:23 ` Markus Armbruster
2014-02-14  2:53   ` Wenchao Xia

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=878uta2le9.fsf@blackfin.pond.sub.org \
    --to=armbru@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=lcapitulino@redhat.com \
    --cc=mdroth@linux.vnet.ibm.com \
    --cc=qemu-devel@nongnu.org \
    --cc=xiawenc@linux.vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.