Re: [Qemu-devel] [PATCH 2/3] qapi-visit: Clean up code generated around visit_end_union()

[Markus Armbruster]

Eric Blake <address@hidden> writes:

> On 01/27/2016 06:54 AM, Markus Armbruster wrote:
>> The generated code can call visit_end_union() without having called
>> visit_start_union().  Example:
>> 
>>      if (!*obj) {
>>          goto out_obj;
>>      }
>>      visit_type_BlockdevOptions_fields(v, obj, &err);
>>      if (err) {
>>          goto out_obj; // if we go from here...
>>      }
>>      if (!visit_start_union(v, !!(*obj)->u.data, &err) || err) {
>>          goto out_obj;
>>      }
>>      switch ((*obj)->driver) {
>>     [...]
>>      }
>>     out_obj:
>>         // ... then *obj is true, and ...
>>      error_propagate(errp, err);
>>      err = NULL;
>>      if (*obj) {
>>          // we end up here
>>          visit_end_union(v, !!(*obj)->u.data, &err);
>>      }
>>      error_propagate(errp, err);

Tabs crept into my commit message, oops.

>> 
>> Harmless only because no visitor implements end_union().  Clean it up
>> anyway.
>
> I plan on deleting visit_end_union() anyways (and visit_start_union);
> see 32/37, plus the FIXME comments added in 21/37.  Maybe it's easier to
> just delete this incorrect and unused callback earlier in the series,
> using your commit message as additional rationale why it is worthless,
> and leaving only visit_start_union() cleanups for 32/37.

I could've tried to move PATCH 32 before the unification, but that
would've been work, so I went with this straightforward fix instead.
Sure, it fixes something that'll go away soon, but I think the churn is
quite tolerable: 1 file changed, 2 insertions(+), 4 deletions(-).

>> Messed up since we have visit_end_union (commit cee2ded).
>
> Indeed.