Eric Blake <ebl...@redhat.com> 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.
