[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v11 03/15] qapi: Forbid 'any' inside an alternat
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v11 03/15] qapi: Forbid 'any' inside an alternate |
Date: |
Fri, 19 Feb 2016 09:32:08 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Eric Blake <address@hidden> writes:
> On 02/18/2016 10:03 AM, Markus Armbruster wrote:
>
>>>> Could use a test for alternate member of alternate type.
>>>
>>> One step ahead of you: commit 3d0c4829 added the test
>>> alternate-nested.json, and commits 44bd1276 and dd883c6f fixed the
>>> parser to reject it (first by a hard-coded check, then via allow_metas[]
>>> excluding alternates). 'any' is the only value that could sneak
>>> through, because it is a subset of 'built-in' which allow_metas[]
>>> whitelisted.
>>
>> Then find_alternate_member_qtype()'s final return None is unreachable,
>> correct?
>
> Indeed, the testsuite still passes with:
>
> diff --git i/scripts/qapi.py w/scripts/qapi.py
> index 8497777..81d435f 100644
> --- i/scripts/qapi.py
> +++ w/scripts/qapi.py
> @@ -345,7 +345,7 @@ def find_alternate_member_qtype(qapi_type):
> return "QTYPE_QSTRING"
> elif find_union(qapi_type):
> return "QTYPE_QDICT"
> - return None
> + assert False
>
>
> # Return the discriminator enum define if discriminator is specified as an
>
>
> That said, even though we currently filter out unknown types before
> deciding to call find_alternate_member_qtype, it's not out of the
> question that future work to move ad hoc front-end tests into formal
> QAPISchema .check() methods may cause us to call
> find_alternate_member_qtype('unknown'). Leaving it as return None
> instead of asserting would make the error message added in this patch
> nicer. Then again, refactoring would move the error message of this
> patch to the .check() methods. So I won't worry about it for now.
Makes sense. I was just making sure I understand how this works.
Thanks!
- Re: [Qemu-devel] [PATCH v11 05/15] qapi-visit: Simplify how we visit common union members, (continued)
[Qemu-devel] [PATCH v11 06/15] qapi: Visit variants in visit_type_FOO_fields(), Eric Blake, 2016/02/18
[Qemu-devel] [PATCH v11 10/15] qapi: Emit structs used as variants in topological order, Eric Blake, 2016/02/18
[Qemu-devel] [PATCH v11 09/15] qapi: Adjust layout of FooList types, Eric Blake, 2016/02/18
[Qemu-devel] [PATCH v11 14/15] qapi: Delete visit_start_union(), gen_visit_implicit_struct(), Eric Blake, 2016/02/18