Re: [Qemu-devel] [PATCH v11 03/15] qapi: Forbid 'any' inside an alternate

[Markus Armbruster]

Eric Blake <address@hidden> writes:

> On 02/18/2016 05:05 AM, Markus Armbruster wrote:
>> Eric Blake <address@hidden> writes:
>> 
>>> The whole point of an alternate is to allow some type-safety while
>>> still accepting more than one JSON type.  Meanwhile, the 'any'
>>> type exists to bypass type-safety altogether.  The two are
>>> incompatible: you can't accept every type, and still tell which
>>> branch of the alternate to use for the parse; fix this to give a
>>> sane error instead of a Python stack trace.
>>>
>>> Signed-off-by: Eric Blake <address@hidden>
>>>
>
>> 
>> Interestingly, find_alternate_member_qtype(T) can return None in two
>> ways: when builtin_types[T] is None (only for T == 'any'), and when T is
>> neither built-in, struct, enum or union (it must be alternate then).
>> 
>> This leads to the question whether this patch catches exactly 'any', as
>> the commit message claims, or alternate as well.
>> 
>
>>> 
>>> @@ -629,7 +629,10 @@ def check_alternate(expr, expr_info):
>>>                     value,
>>>                     allow_metas=['built-in', 'union', 'struct', 'enum'])
>>>          qtype = find_alternate_member_qtype(value)
>> 
>
>> 
>> 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?

> If you want to squash any of this information into the commit message,
> though, I don't mind.

I'll consider it when I apply.