[Qemu-devel] [PATCH for-2.9 09/47] qapi: Fix to reject empty union base gracefully

Markus Armbruster posted 47 patches 8 years, 7 months ago
There is a newer version of this series
[Qemu-devel] [PATCH for-2.9 09/47] qapi: Fix to reject empty union base gracefully
Posted by Markus Armbruster 8 years, 7 months ago
Common Python pitfall: 'assert base_members' fires on [] in addition
to None.  Correct to 'assert base_members is not None'.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/qapi.py                        |  2 +-
 tests/qapi-schema/union-base-empty.err | 11 +----------
 2 files changed, 2 insertions(+), 11 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index e98fd0c..eec7bfb 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -731,7 +731,7 @@ def check_union(expr, info):
             raise QAPISemError(info, "Flat union '%s' must have a base"
                                % name)
         base_members = find_base_members(base)
-        assert base_members
+        assert base_members is not None
 
         # The value of member 'discriminator' must name a non-optional
         # member of the base struct.
diff --git a/tests/qapi-schema/union-base-empty.err b/tests/qapi-schema/union-base-empty.err
index 61e6ec6..7695806 100644
--- a/tests/qapi-schema/union-base-empty.err
+++ b/tests/qapi-schema/union-base-empty.err
@@ -1,10 +1 @@
-Traceback (most recent call last):
-  File "tests/qapi-schema/test-qapi.py", line 56, in <module>
-    schema = QAPISchema(sys.argv[1])
-  File "scripts/qapi.py", line 1483, in __init__
-    self.exprs = check_exprs(parser.exprs)
-  File "scripts/qapi.py", line 917, in check_exprs
-    check_union(expr, info)
-  File "scripts/qapi.py", line 734, in check_union
-    assert base_members
-AssertionError
+tests/qapi-schema/union-base-empty.json:5: Discriminator 'type' is not a member of base struct 'Empty'
-- 
2.7.4


Re: [Qemu-devel] [PATCH for-2.9 09/47] qapi: Fix to reject empty union base gracefully
Posted by Marc-André Lureau 8 years, 7 months ago
Hi

On Mon, Mar 13, 2017 at 10:19 AM Markus Armbruster <armbru@redhat.com>
wrote:

> Common Python pitfall: 'assert base_members' fires on [] in addition
> to None.  Correct to 'assert base_members is not None'.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


> ---
>  scripts/qapi.py                        |  2 +-
>  tests/qapi-schema/union-base-empty.err | 11 +----------
>  2 files changed, 2 insertions(+), 11 deletions(-)
>
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index e98fd0c..eec7bfb 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -731,7 +731,7 @@ def check_union(expr, info):
>              raise QAPISemError(info, "Flat union '%s' must have a base"
>                                 % name)
>          base_members = find_base_members(base)
> -        assert base_members
> +        assert base_members is not None
>
>          # The value of member 'discriminator' must name a non-optional
>          # member of the base struct.
> diff --git a/tests/qapi-schema/union-base-empty.err
> b/tests/qapi-schema/union-base-empty.err
> index 61e6ec6..7695806 100644
> --- a/tests/qapi-schema/union-base-empty.err
> +++ b/tests/qapi-schema/union-base-empty.err
> @@ -1,10 +1 @@
> -Traceback (most recent call last):
> -  File "tests/qapi-schema/test-qapi.py", line 56, in <module>
> -    schema = QAPISchema(sys.argv[1])
> -  File "scripts/qapi.py", line 1483, in __init__
> -    self.exprs = check_exprs(parser.exprs)
> -  File "scripts/qapi.py", line 917, in check_exprs
> -    check_union(expr, info)
> -  File "scripts/qapi.py", line 734, in check_union
> -    assert base_members
> -AssertionError
> +tests/qapi-schema/union-base-empty.json:5: Discriminator 'type' is not a
> member of base struct 'Empty'
> --
> 2.7.4
>
>
> --
Marc-André Lureau
Re: [Qemu-devel] [PATCH for-2.9 09/47] qapi: Fix to reject empty union base gracefully
Posted by Eric Blake 8 years, 7 months ago
On 03/13/2017 01:18 AM, Markus Armbruster wrote:
> Common Python pitfall: 'assert base_members' fires on [] in addition
> to None.  Correct to 'assert base_members is not None'.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  scripts/qapi.py                        |  2 +-
>  tests/qapi-schema/union-base-empty.err | 11 +----------
>  2 files changed, 2 insertions(+), 11 deletions(-)
> 

> +++ b/tests/qapi-schema/union-base-empty.err
> @@ -1,10 +1 @@
> -Traceback (most recent call last):
> -  File "tests/qapi-schema/test-qapi.py", line 56, in <module>
> -    schema = QAPISchema(sys.argv[1])
> -  File "scripts/qapi.py", line 1483, in __init__
> -    self.exprs = check_exprs(parser.exprs)
> -  File "scripts/qapi.py", line 917, in check_exprs
> -    check_union(expr, info)
> -  File "scripts/qapi.py", line 734, in check_union
> -    assert base_members
> -AssertionError
> +tests/qapi-schema/union-base-empty.json:5: Discriminator 'type' is not a member of base struct 'Empty'

Much nicer message.
Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org