Re: [Qemu-devel] [PATCH v10 12/13] qapi: Delete unused visit_start_union()

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1507 bytes --]

On 02/18/2016 01:24 AM, Markus Armbruster wrote:
>> For simple unions, you could have (well, still can have, until my later
>> patch gets rid of the simple_union_type() magic):
>>
>> struct SU {
>>     SUKind type;
>>     union {
>>         void *data;
>>         int8_t byte;
>>     } u;
>> };
> 
> Begs the question why that works :)

By sheer luck, and (poorly?) documented in a hairy comment in
qapi-dealloc-visitor.c (at least, until I delete visit_start_union).  We
have a data-dependent decision (not only the contents of 'byte', but
ALSO the contents of the padding bits), but either the decision results
in calling visit_type_int8() (and doing nothing) or skipping the call
(and likewise doing nothing).


>> I guess I may try and reword this slightly, and point to the fact that
>> the NULL dereference was due to calling visit_start_implicit_FOO() (only
>> done for flat unions; for simple unions the branches call
>> visit_type_FOO(), and that call safely handled NULL);
> 
> That's why it works?
> 

>> But now that visit_type_implicit_FOO() is gone, my earlier incantation
>> got reduced in size.  I guess it's all in how I document the commit message.
> 
> Give it a try :)

I gave it my best in v11 :)  Maybe you'll still have wording
improvements, but this back-and-forth has helped both of us try to
actually characterize what is going on.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]