diff mbox

[V6,02/10] qapi script: add check for duplicated key

Message ID 1392068921-3327-3-git-send-email-xiawenc@linux.vnet.ibm.com
State New
Headers show

Commit Message

Wayne Xia Feb. 10, 2014, 9:48 p.m. UTC 
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(-)

Comments

Markus Armbruster Feb. 13, 2014, 3:14 p.m. UTC | #1 
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.
Wayne Xia Feb. 14, 2014, 1:50 a.m. UTC | #2 
于 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.
diff mbox

Patch

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()