Re: [Qemu-devel] [PATCH v2 01/54] qapi: fix type_seen key error

2017-08-28 Thread Markus Armbruster 
Marc-André Lureau  writes:

> On Fri, Aug 25, 2017 at 2:57 PM Eduardo Habkost  wrote:
>
>> On Fri, Aug 25, 2017 at 08:02:26AM +0200, Markus Armbruster wrote:
>> > Conflicts with Eduardo's "[PATCH v2] qapi: Fix error handling code on
>> > alternate conflict".
>> > Message-Id: <20170717180926.14924-1-ehabk...@redhat.com>
>> >
>> > Marc-André, could you have a look?  You can rebase your fix on top of
>> > Eduardo's, or merge the two into one commit.
>>
>> Both patches seem to be fixing the same bug.
>>
>
> Yes, it is solved differently. But take Eduardo's patch, it has more
> extensive tests .

Okay.

Too bad I didn't remember Eduardo's patch when I got yours.

Re: [Qemu-devel] [PATCH v2 01/54] qapi: fix type_seen key error

2017-08-25 Thread Marc-André Lureau 
On Fri, Aug 25, 2017 at 2:57 PM Eduardo Habkost  wrote:

> On Fri, Aug 25, 2017 at 08:02:26AM +0200, Markus Armbruster wrote:
> > Conflicts with Eduardo's "[PATCH v2] qapi: Fix error handling code on
> > alternate conflict".
> > Message-Id: <20170717180926.14924-1-ehabk...@redhat.com>
> >
> > Marc-André, could you have a look?  You can rebase your fix on top of
> > Eduardo's, or merge the two into one commit.
>
> Both patches seem to be fixing the same bug.
>

Yes, it is solved differently. But take Eduardo's patch, it has more
extensive tests .

>
> >
> > Marc-André Lureau  writes:
> >
> > > The type_seen member can be of a different type than the 'qtype' being
> > > checked, since a string create several conflicts. Lookup the real
> > > conflicting type in the conflict set, that one must be present in
> > > type_seen.
> > >
> > > This fixes the following error, reproducible with the modified test:
> > >
> > > Traceback (most recent call last):
> > >   File "/home/elmarco/src/qq/tests/qapi-schema/test-qapi.py", line 56,
> in 
> > > schema = QAPISchema(sys.argv[1])
> > >   File "/home/elmarco/src/qq/scripts/qapi.py", line 1470, in __init__
> > > self.exprs = check_exprs(parser.exprs)
> > >   File "/home/elmarco/src/qq/scripts/qapi.py", line 959, in check_exprs
> > > check_alternate(expr, info)
> > >   File "/home/elmarco/src/qq/scripts/qapi.py", line 831, in
> check_alternate
> > > % (name, key, types_seen[qtype]))
> > > KeyError: 'QTYPE_QSTRING'
> > >
> > > Signed-off-by: Marc-André Lureau 
> > > ---
> > >  scripts/qapi.py  | 6 --
> > >  tests/qapi-schema/alternate-conflict-string.json | 4 ++--
> > >  2 files changed, 6 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/scripts/qapi.py b/scripts/qapi.py
> > > index 8aa2775f12..a3ac799535 100644
> > > --- a/scripts/qapi.py
> > > +++ b/scripts/qapi.py
> > > @@ -825,10 +825,12 @@ def check_alternate(expr, info):
> > >  else:
> > >  conflicting.add('QTYPE_QNUM')
> > >  conflicting.add('QTYPE_QBOOL')
> > > -if conflicting & set(types_seen):
> > > +conflict = conflicting & set(types_seen)
> > > +if conflict:
> > > +conflict_qtype = list(conflict)[0]
> > >  raise QAPISemError(info, "Alternate '%s' member '%s'
> can't "
> > > "be distinguished from member '%s'"
> > > -   % (name, key, types_seen[qtype]))
> > > +   % (name, key,
> types_seen[conflict_qtype]))
> > >  for qt in conflicting:
> > >  types_seen[qt] = key
> > >
> > > diff --git a/tests/qapi-schema/alternate-conflict-string.json
> b/tests/qapi-schema/alternate-conflict-string.json
> > > index 85adbd4adc..bb2702978e 100644
> > > --- a/tests/qapi-schema/alternate-conflict-string.json
> > > +++ b/tests/qapi-schema/alternate-conflict-string.json
> > > @@ -1,4 +1,4 @@
> > >  # alternate branches of 'str' type conflict with all scalar types
> > >  { 'alternate': 'Alt',
> > > -  'data': { 'one': 'str',
> > > -'two': 'int' } }
> > > +  'data': { 'one': 'int',
> > > +'two': 'str' } }
>
> --
> Eduardo
>
> --
Marc-André Lureau

Re: [Qemu-devel] [PATCH v2 01/54] qapi: fix type_seen key error

2017-08-25 Thread Eduardo Habkost 
On Fri, Aug 25, 2017 at 08:02:26AM +0200, Markus Armbruster wrote:
> Conflicts with Eduardo's "[PATCH v2] qapi: Fix error handling code on
> alternate conflict".
> Message-Id: <20170717180926.14924-1-ehabk...@redhat.com>
> 
> Marc-André, could you have a look?  You can rebase your fix on top of
> Eduardo's, or merge the two into one commit.

Both patches seem to be fixing the same bug.

> 
> Marc-André Lureau  writes:
> 
> > The type_seen member can be of a different type than the 'qtype' being
> > checked, since a string create several conflicts. Lookup the real
> > conflicting type in the conflict set, that one must be present in
> > type_seen.
> >
> > This fixes the following error, reproducible with the modified test:
> >
> > Traceback (most recent call last):
> >   File "/home/elmarco/src/qq/tests/qapi-schema/test-qapi.py", line 56, in 
> > 
> > schema = QAPISchema(sys.argv[1])
> >   File "/home/elmarco/src/qq/scripts/qapi.py", line 1470, in __init__
> > self.exprs = check_exprs(parser.exprs)
> >   File "/home/elmarco/src/qq/scripts/qapi.py", line 959, in check_exprs
> > check_alternate(expr, info)
> >   File "/home/elmarco/src/qq/scripts/qapi.py", line 831, in check_alternate
> > % (name, key, types_seen[qtype]))
> > KeyError: 'QTYPE_QSTRING'
> >
> > Signed-off-by: Marc-André Lureau 
> > ---
> >  scripts/qapi.py  | 6 --
> >  tests/qapi-schema/alternate-conflict-string.json | 4 ++--
> >  2 files changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/scripts/qapi.py b/scripts/qapi.py
> > index 8aa2775f12..a3ac799535 100644
> > --- a/scripts/qapi.py
> > +++ b/scripts/qapi.py
> > @@ -825,10 +825,12 @@ def check_alternate(expr, info):
> >  else:
> >  conflicting.add('QTYPE_QNUM')
> >  conflicting.add('QTYPE_QBOOL')
> > -if conflicting & set(types_seen):
> > +conflict = conflicting & set(types_seen)
> > +if conflict:
> > +conflict_qtype = list(conflict)[0]
> >  raise QAPISemError(info, "Alternate '%s' member '%s' can't "
> > "be distinguished from member '%s'"
> > -   % (name, key, types_seen[qtype]))
> > +   % (name, key, types_seen[conflict_qtype]))
> >  for qt in conflicting:
> >  types_seen[qt] = key
> >  
> > diff --git a/tests/qapi-schema/alternate-conflict-string.json 
> > b/tests/qapi-schema/alternate-conflict-string.json
> > index 85adbd4adc..bb2702978e 100644
> > --- a/tests/qapi-schema/alternate-conflict-string.json
> > +++ b/tests/qapi-schema/alternate-conflict-string.json
> > @@ -1,4 +1,4 @@
> >  # alternate branches of 'str' type conflict with all scalar types
> >  { 'alternate': 'Alt',
> > -  'data': { 'one': 'str',
> > -'two': 'int' } }
> > +  'data': { 'one': 'int',
> > +'two': 'str' } }

-- 
Eduardo

Re: [Qemu-devel] [PATCH v2 01/54] qapi: fix type_seen key error

2017-08-25 Thread Markus Armbruster 
Conflicts with Eduardo's "[PATCH v2] qapi: Fix error handling code on
alternate conflict".
Message-Id: <20170717180926.14924-1-ehabk...@redhat.com>

Marc-André, could you have a look?  You can rebase your fix on top of
Eduardo's, or merge the two into one commit.

Marc-André Lureau  writes:

> The type_seen member can be of a different type than the 'qtype' being
> checked, since a string create several conflicts. Lookup the real
> conflicting type in the conflict set, that one must be present in
> type_seen.
>
> This fixes the following error, reproducible with the modified test:
>
> Traceback (most recent call last):
>   File "/home/elmarco/src/qq/tests/qapi-schema/test-qapi.py", line 56, in 
> 
> schema = QAPISchema(sys.argv[1])
>   File "/home/elmarco/src/qq/scripts/qapi.py", line 1470, in __init__
> self.exprs = check_exprs(parser.exprs)
>   File "/home/elmarco/src/qq/scripts/qapi.py", line 959, in check_exprs
> check_alternate(expr, info)
>   File "/home/elmarco/src/qq/scripts/qapi.py", line 831, in check_alternate
> % (name, key, types_seen[qtype]))
> KeyError: 'QTYPE_QSTRING'
>
> Signed-off-by: Marc-André Lureau 
> ---
>  scripts/qapi.py  | 6 --
>  tests/qapi-schema/alternate-conflict-string.json | 4 ++--
>  2 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index 8aa2775f12..a3ac799535 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -825,10 +825,12 @@ def check_alternate(expr, info):
>  else:
>  conflicting.add('QTYPE_QNUM')
>  conflicting.add('QTYPE_QBOOL')
> -if conflicting & set(types_seen):
> +conflict = conflicting & set(types_seen)
> +if conflict:
> +conflict_qtype = list(conflict)[0]
>  raise QAPISemError(info, "Alternate '%s' member '%s' can't "
> "be distinguished from member '%s'"
> -   % (name, key, types_seen[qtype]))
> +   % (name, key, types_seen[conflict_qtype]))
>  for qt in conflicting:
>  types_seen[qt] = key
>  
> diff --git a/tests/qapi-schema/alternate-conflict-string.json 
> b/tests/qapi-schema/alternate-conflict-string.json
> index 85adbd4adc..bb2702978e 100644
> --- a/tests/qapi-schema/alternate-conflict-string.json
> +++ b/tests/qapi-schema/alternate-conflict-string.json
> @@ -1,4 +1,4 @@
>  # alternate branches of 'str' type conflict with all scalar types
>  { 'alternate': 'Alt',
> -  'data': { 'one': 'str',
> -'two': 'int' } }
> +  'data': { 'one': 'int',
> +'two': 'str' } }

Re: [Qemu-devel] [PATCH v2 01/54] qapi: fix type_seen key error

2017-08-22 Thread Markus Armbruster 
Marc-André Lureau  writes:

> Hi
>
> - Original Message -
>> Marc-André Lureau  writes:
>> 
>> > The type_seen member can be of a different type than the 'qtype' being
>> > checked, since a string create several conflicts. Lookup the real
>> > conflicting type in the conflict set, that one must be present in
>> > type_seen.
>> >
>> > This fixes the following error, reproducible with the modified test:
>> >
>> > Traceback (most recent call last):
>> >   File "/home/elmarco/src/qq/tests/qapi-schema/test-qapi.py", line 56, in
>> >   
>> > schema = QAPISchema(sys.argv[1])
>> >   File "/home/elmarco/src/qq/scripts/qapi.py", line 1470, in __init__
>> > self.exprs = check_exprs(parser.exprs)
>> >   File "/home/elmarco/src/qq/scripts/qapi.py", line 959, in check_exprs
>> > check_alternate(expr, info)
>> >   File "/home/elmarco/src/qq/scripts/qapi.py", line 831, in check_alternate
>> > % (name, key, types_seen[qtype]))
>> > KeyError: 'QTYPE_QSTRING'
>> >
>> > Signed-off-by: Marc-André Lureau 
>> > ---
>> >  scripts/qapi.py  | 6 --
>> >  tests/qapi-schema/alternate-conflict-string.json | 4 ++--
>> >  2 files changed, 6 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/scripts/qapi.py b/scripts/qapi.py
>> > index 8aa2775f12..a3ac799535 100644
>> > --- a/scripts/qapi.py
>> > +++ b/scripts/qapi.py
>> > @@ -825,10 +825,12 @@ def check_alternate(expr, info):
>> >  else:
>> >  conflicting.add('QTYPE_QNUM')
>> >  conflicting.add('QTYPE_QBOOL')
>> > -if conflicting & set(types_seen):
>> > +conflict = conflicting & set(types_seen)
>> > +if conflict:
>> > +conflict_qtype = list(conflict)[0]
>> 
>> Converting from set to list just to extract an element is clumsly.
>> Let's use conflict.pop(), and eliminate the variable.  Can do on commit.
>
> I didn't realize that. thanks

I didn't know about .pop() either, but I went "there's *got* to be a
better way!" and searched the docs for it :)

[...]

Re: [Qemu-devel] [PATCH v2 01/54] qapi: fix type_seen key error

2017-08-22 Thread Marc-André Lureau 
Hi

- Original Message -
> Marc-André Lureau  writes:
> 
> > The type_seen member can be of a different type than the 'qtype' being
> > checked, since a string create several conflicts. Lookup the real
> > conflicting type in the conflict set, that one must be present in
> > type_seen.
> >
> > This fixes the following error, reproducible with the modified test:
> >
> > Traceback (most recent call last):
> >   File "/home/elmarco/src/qq/tests/qapi-schema/test-qapi.py", line 56, in
> >   
> > schema = QAPISchema(sys.argv[1])
> >   File "/home/elmarco/src/qq/scripts/qapi.py", line 1470, in __init__
> > self.exprs = check_exprs(parser.exprs)
> >   File "/home/elmarco/src/qq/scripts/qapi.py", line 959, in check_exprs
> > check_alternate(expr, info)
> >   File "/home/elmarco/src/qq/scripts/qapi.py", line 831, in check_alternate
> > % (name, key, types_seen[qtype]))
> > KeyError: 'QTYPE_QSTRING'
> >
> > Signed-off-by: Marc-André Lureau 
> > ---
> >  scripts/qapi.py  | 6 --
> >  tests/qapi-schema/alternate-conflict-string.json | 4 ++--
> >  2 files changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/scripts/qapi.py b/scripts/qapi.py
> > index 8aa2775f12..a3ac799535 100644
> > --- a/scripts/qapi.py
> > +++ b/scripts/qapi.py
> > @@ -825,10 +825,12 @@ def check_alternate(expr, info):
> >  else:
> >  conflicting.add('QTYPE_QNUM')
> >  conflicting.add('QTYPE_QBOOL')
> > -if conflicting & set(types_seen):
> > +conflict = conflicting & set(types_seen)
> > +if conflict:
> > +conflict_qtype = list(conflict)[0]
> 
> Converting from set to list just to extract an element is clumsly.
> Let's use conflict.pop(), and eliminate the variable.  Can do on commit.

I didn't realize that. thanks

> 
> >  raise QAPISemError(info, "Alternate '%s' member '%s' can't "
> > "be distinguished from member '%s'"
> > -   % (name, key, types_seen[qtype]))
> > +   % (name, key, types_seen[conflict_qtype]))
> >  for qt in conflicting:
> >  types_seen[qt] = key
> >  
> > diff --git a/tests/qapi-schema/alternate-conflict-string.json
> > b/tests/qapi-schema/alternate-conflict-string.json
> > index 85adbd4adc..bb2702978e 100644
> > --- a/tests/qapi-schema/alternate-conflict-string.json
> > +++ b/tests/qapi-schema/alternate-conflict-string.json
> > @@ -1,4 +1,4 @@
> >  # alternate branches of 'str' type conflict with all scalar types
> >  { 'alternate': 'Alt',
> > -  'data': { 'one': 'str',
> > -'two': 'int' } }
> > +  'data': { 'one': 'int',
> > +'two': 'str' } }
> 
> Reviewed-by: Markus Armbruster 
>

Re: [Qemu-devel] [PATCH v2 01/54] qapi: fix type_seen key error

2017-08-22 Thread Markus Armbruster 
Marc-André Lureau  writes:

> The type_seen member can be of a different type than the 'qtype' being
> checked, since a string create several conflicts. Lookup the real
> conflicting type in the conflict set, that one must be present in
> type_seen.
>
> This fixes the following error, reproducible with the modified test:
>
> Traceback (most recent call last):
>   File "/home/elmarco/src/qq/tests/qapi-schema/test-qapi.py", line 56, in 
> 
> schema = QAPISchema(sys.argv[1])
>   File "/home/elmarco/src/qq/scripts/qapi.py", line 1470, in __init__
> self.exprs = check_exprs(parser.exprs)
>   File "/home/elmarco/src/qq/scripts/qapi.py", line 959, in check_exprs
> check_alternate(expr, info)
>   File "/home/elmarco/src/qq/scripts/qapi.py", line 831, in check_alternate
> % (name, key, types_seen[qtype]))
> KeyError: 'QTYPE_QSTRING'
>
> Signed-off-by: Marc-André Lureau 
> ---
>  scripts/qapi.py  | 6 --
>  tests/qapi-schema/alternate-conflict-string.json | 4 ++--
>  2 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index 8aa2775f12..a3ac799535 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -825,10 +825,12 @@ def check_alternate(expr, info):
>  else:
>  conflicting.add('QTYPE_QNUM')
>  conflicting.add('QTYPE_QBOOL')
> -if conflicting & set(types_seen):
> +conflict = conflicting & set(types_seen)
> +if conflict:
> +conflict_qtype = list(conflict)[0]

Converting from set to list just to extract an element is clumsly.
Let's use conflict.pop(), and eliminate the variable.  Can do on commit.

>  raise QAPISemError(info, "Alternate '%s' member '%s' can't "
> "be distinguished from member '%s'"
> -   % (name, key, types_seen[qtype]))
> +   % (name, key, types_seen[conflict_qtype]))
>  for qt in conflicting:
>  types_seen[qt] = key
>  
> diff --git a/tests/qapi-schema/alternate-conflict-string.json 
> b/tests/qapi-schema/alternate-conflict-string.json
> index 85adbd4adc..bb2702978e 100644
> --- a/tests/qapi-schema/alternate-conflict-string.json
> +++ b/tests/qapi-schema/alternate-conflict-string.json
> @@ -1,4 +1,4 @@
>  # alternate branches of 'str' type conflict with all scalar types
>  { 'alternate': 'Alt',
> -  'data': { 'one': 'str',
> -'two': 'int' } }
> +  'data': { 'one': 'int',
> +'two': 'str' } }

Reviewed-by: Markus Armbruster

[Qemu-devel] [PATCH v2 01/54] qapi: fix type_seen key error

2017-08-22 Thread Marc-André Lureau 
The type_seen member can be of a different type than the 'qtype' being
checked, since a string create several conflicts. Lookup the real
conflicting type in the conflict set, that one must be present in
type_seen.

This fixes the following error, reproducible with the modified test:

Traceback (most recent call last):
  File "/home/elmarco/src/qq/tests/qapi-schema/test-qapi.py", line 56, in 

schema = QAPISchema(sys.argv[1])
  File "/home/elmarco/src/qq/scripts/qapi.py", line 1470, in __init__
self.exprs = check_exprs(parser.exprs)
  File "/home/elmarco/src/qq/scripts/qapi.py", line 959, in check_exprs
check_alternate(expr, info)
  File "/home/elmarco/src/qq/scripts/qapi.py", line 831, in check_alternate
% (name, key, types_seen[qtype]))
KeyError: 'QTYPE_QSTRING'

Signed-off-by: Marc-André Lureau 
---
 scripts/qapi.py  | 6 --
 tests/qapi-schema/alternate-conflict-string.json | 4 ++--
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 8aa2775f12..a3ac799535 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -825,10 +825,12 @@ def check_alternate(expr, info):
 else:
 conflicting.add('QTYPE_QNUM')
 conflicting.add('QTYPE_QBOOL')
-if conflicting & set(types_seen):
+conflict = conflicting & set(types_seen)
+if conflict:
+conflict_qtype = list(conflict)[0]
 raise QAPISemError(info, "Alternate '%s' member '%s' can't "
"be distinguished from member '%s'"
-   % (name, key, types_seen[qtype]))
+   % (name, key, types_seen[conflict_qtype]))
 for qt in conflicting:
 types_seen[qt] = key
 
diff --git a/tests/qapi-schema/alternate-conflict-string.json 
b/tests/qapi-schema/alternate-conflict-string.json
index 85adbd4adc..bb2702978e 100644
--- a/tests/qapi-schema/alternate-conflict-string.json
+++ b/tests/qapi-schema/alternate-conflict-string.json
@@ -1,4 +1,4 @@
 # alternate branches of 'str' type conflict with all scalar types
 { 'alternate': 'Alt',
-  'data': { 'one': 'str',
-'two': 'int' } }
+  'data': { 'one': 'int',
+'two': 'str' } }
-- 
2.14.1.146.gd35faa819