Re: [Qemu-devel] [PATCH RFC v2 27/47] qapi-visit: Convert to QAPISchemaVisitor, fixing bugs

[Markus Armbruster]

Eric Blake <address@hidden> writes:

> On 07/01/2015 02:22 PM, Markus Armbruster wrote:
>> Fixes flat unions to visit the base's base members (the previous
>> commit merely added them to the struct).  Same test case.
>> 
>> Patch's effect on visit_type_UserDefFlatUnion():
>> 
>>      static void visit_type_UserDefFlatUnion_fields(Visitor *m, 
>> UserDefFlatUnion **obj, Error **errp)
>>      {
>>          Error *err = NULL;
>> 
>>     +    visit_type_int(m, &(*obj)->integer, "integer", &err);
>>     +    if (err) {
>>     +        goto out;
>>     +    }
>>          visit_type_str(m, &(*obj)->string, "string", &err);
>>          if (err) {
>>              goto out;
>> 
>
>> +def gen_visit_union(name, base, variants):
>> +    ret = ''
>>  
>>      if base:
>> -        assert discriminator
>> -        base_fields = find_struct(base)['data'].copy()
>> -        del base_fields[discriminator]
>> -        ret += generate_visit_struct_fields(name, base_fields)
>> +        members = [m for m in base.members if m != variants.tag_member]
>> +        ret += generate_visit_struct_fields(name, members)
>
> Ouch. This hurts.  If the same class is used as both the base class of a
> flat union, and the base class of an ordinary struct, then the struct
> tries to visit the base class, but no longer parses the field that the
> union was using as its discriminator.
>
> We don't have any code that demonstrates this, but probably should.  I
> ran into it while working up my POC of what it would take to unbox
> inherited structs (http://thread.gmane.org/gmane.comp.emulators.qemu/353204)

Is this broken in master, or do my patches break it?

Got a reproducer?

[...]