Re: [Qemu-devel] [PATCH v5 07/46] qapi: Don't pass pre-existing error to later call

[Eric Blake]

On 09/24/2015 08:58 AM, Markus Armbruster wrote:
> Eric Blake <address@hidden> writes:
> 
>> Due to the existing semantics of the error_set() family,
>> generated sequences in the qapi visitors such as:
>>
>>     visit_start_implicit_struct(m, (void **)obj, sizeof(FOO), &err);
>>         if (!err) {
>>             visit_type_FOO_fields(m, obj, errp);
>>             visit_end_implicit_struct(m, &err);
>>         }
>>     error_propagate(errp, err);
> 
> Indentation seems off.  Intentional?
> 
>>
>> are risky: if visit_type_FOO_fields() sets errp, and then
>> visit_end_implicit_struct() also encounters an error, the
>> attempt to overwrite the first error will cause an abort().

I didn't even read error_propagate()'s contract correctly. It
specifically specifies that if errp is already set, then err is ignored.

So the above sequence is actually just fine, because only the following
paths exist:

visit_start_implicit_struct() fails into &err,
error_propagate() passes err into caller's errp

visit_start_implicit_struct() succeeds,
visit_type_FOO_fields() fails into caller's errp,
visit_end_implicit_struct() succeeds,
error_propagate() does nothing

visit_start_implicit_struct() succeeds,
visit_type_FOO_fields() fails into caller's errp,
visit_end_implicit_struct() fails int &err,
error_propagate() does nothing (errp trumps err)

visit_start_implicit_struct() succeeds,
visit_type_FOO_fields() succeeds,
visit_end_implicit_struct() fails int &err,
error_propagate() passes err into caller's errp

visit_start_implicit_struct() succeeds,
visit_type_FOO_fields() succeeds,
visit_end_implicit_struct() succeeds,
error_propagate() does nothing


As such, I'm revisiting if anything is needed at all, other than making
the various visit_start/visit_end patterns consistent with one another
using existing idioms, and it may turn out we don't need the ternary
after all.

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

signature.asc
Description: OpenPGP digital signature