diff mbox

[v9,14/27] qapi: Fix up commit 7618b91's clash sanity checking change

Message ID 1446618049-13596-15-git-send-email-eblake@redhat.com
State New
Headers show

Commit Message

Eric Blake Nov. 4, 2015, 6:20 a.m. UTC
From: Markus Armbruster <armbru@redhat.com>

This hunk

    @@ -964,6 +965,7 @@ class QAPISchemaObjectType(QAPISchemaType):
                 members = []
             seen = {}
             for m in members:
    +            assert c_name(m.name) not in seen
                 seen[m.name] = m
             for m in self.local_members:
                 m.check(schema, members, seen)

is plainly broken.

Asserting the members inherited from base don't clash is somewhat
redundant, because self.base.check() just checked that.  But it
doesn't hurt.

The idea to use c_name(m.name) instead of m.name for collision
checking is sound, because we need to catch clashes between the m.name
and between the c_name(m.name), and when two m.name clash, then their
c_name() also clash.

However, using c_name(m.name) instead of m.name in one of several
places doesn't work.  See the very next line.

Keep the assertion, but drop the c_name() for now.  A future commit
will bring it back.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1446559499-26984-4-git-send-email-armbru@redhat.com>
[change TAB to space]
Signed-off-by: Eric Blake <eblake@redhat.com>

---
v9: new patch
---
 scripts/qapi.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Markus Armbruster Nov. 4, 2015, 1:36 p.m. UTC | #1
Eric Blake <eblake@redhat.com> writes:

> From: Markus Armbruster <armbru@redhat.com>
>
> This hunk
>
>     @@ -964,6 +965,7 @@ class QAPISchemaObjectType(QAPISchemaType):
>                  members = []
>              seen = {}
>              for m in members:
>     +            assert c_name(m.name) not in seen
>                  seen[m.name] = m
>              for m in self.local_members:
>                  m.check(schema, members, seen)
>
> is plainly broken.
>
> Asserting the members inherited from base don't clash is somewhat
> redundant, because self.base.check() just checked that.  But it
> doesn't hurt.
>
> The idea to use c_name(m.name) instead of m.name for collision
> checking is sound, because we need to catch clashes between the m.name
> and between the c_name(m.name), and when two m.name clash, then their
> c_name() also clash.
>
> However, using c_name(m.name) instead of m.name in one of several
> places doesn't work.  See the very next line.
>
> Keep the assertion, but drop the c_name() for now.  A future commit
> will bring it back.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> Message-Id: <1446559499-26984-4-git-send-email-armbru@redhat.com>
> [change TAB to space]

In the commit message.  You had me wondering briefly whether my Emacs
configuration to avoid tabs in QEMU code failed :)

> Signed-off-by: Eric Blake <eblake@redhat.com>
>
> ---
> v9: new patch
> ---
>  scripts/qapi.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index 145dbfe..c910715 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -987,7 +987,7 @@ class QAPISchemaObjectType(QAPISchemaType):
>              members = []
>          seen = {}
>          for m in members:
> -            assert c_name(m.name) not in seen
> +            assert m.name not in seen
>              seen[m.name] = m
>          for m in self.local_members:
>              m.check(schema, members, seen)
diff mbox

Patch

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 145dbfe..c910715 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -987,7 +987,7 @@  class QAPISchemaObjectType(QAPISchemaType):
             members = []
         seen = {}
         for m in members:
-            assert c_name(m.name) not in seen
+            assert m.name not in seen
             seen[m.name] = m
         for m in self.local_members:
             m.check(schema, members, seen)