[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5 10/28] qapi: Segregate anonymous unions into
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v5 10/28] qapi: Segregate anonymous unions into alternates in generator |
Date: |
Thu, 26 Mar 2015 09:26:36 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 |
On 03/26/2015 08:47 AM, Markus Armbruster wrote:
> Eric Blake <address@hidden> writes:
>
>> Special-casing 'discriminator == {}' for handling anonymous unions
>> is getting awkward; since this particular type is not always a
>> dictionary on the wire, it is easier to treat it as a completely
>> different class of type, "alternate", so that if a type is listed
>> in the union_types array, we know it is not an anonymous union.
>>
>> This patch just further segregates union handling, to make sure that
>> anonymous unions are not stored in union_types, and splitting up
>> check_union() into separate functions. A future patch will change
>> the qapi grammar, and having the segregation already in place will
>> make it easier to deal with the distinct meta-type.
>>
>> Signed-off-by: Eric Blake <address@hidden>
>> ---
>> @@ -535,7 +546,8 @@ def find_struct(name):
>>
>> def add_union(definition):
>> global union_types
>> - union_types.append(definition)
>> + if definition.get('discriminator') != {}:
>> + union_types.append(definition)
>>
>> def find_union(name):
>> global union_types
>
> This is the only unobvious hunk.
>
> union_types is used only through find_union. The hunk makes
> find_union(N) return None when N names an anonymous union.
>
> find_union() is used in two places:
>
> * find_alternate_member_qtype()
>
> Patched further up. It really wants only non-anonymous unions, and
> this change to find_union() renders its check for anonymous unions
> superfluous. Good.
>
> * generate_visit_alternate()
>
> Asserts that each member's type is either a built-in type, a complex
> type, a union type, or an enum type.
>
> The change relaxes the assertion not to trigger on anonymous union
> types. Why is that okay?
No, the change tightens the assertion so that it will now fail on an
anonymous union nested as a branch of another anonymous union (where
before it could silently pass the assertion), because the anonymous
union is no longer found by find_union(). And this is okay because the
earlier change to find_alternate_member_qtype means that we don't allow
nested anonymous unions, so making the assertion fail if an anonymous
union gets through anyway is correct.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
[Qemu-devel] [PATCH v5 03/28] qapi: Require ASCII in schema, Eric Blake, 2015/03/24
[Qemu-devel] [PATCH v5 10/28] qapi: Segregate anonymous unions into alternates in generator, Eric Blake, 2015/03/24
[Qemu-devel] [PATCH v5 08/28] qapi: Better error messages for bad unions, Eric Blake, 2015/03/24
[Qemu-devel] [PATCH v5 04/28] qapi: Add some enum tests, Eric Blake, 2015/03/24
[Qemu-devel] [PATCH v5 07/28] qapi: Simplify tests of simple unions, Eric Blake, 2015/03/24
[Qemu-devel] [PATCH v5 09/28] qapi: Prepare for catching more semantic parse errors, Eric Blake, 2015/03/24