On 09/26/2015 03:41 PM, Eric Blake wrote: > On 09/24/2015 08:58 AM, Markus Armbruster wrote: >> Eric Blake <ebl...@redhat.com> 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: >
> > 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. Turns out patch 29/46 needs the ternary. There, I'm changing the logic of the various visit_type_FOO() to explicitly set *obj = NULL if something fails in between visit_start_* and visit_end_* - but to do that, I _have_ to track everything locally in &err (since errp might be NULL in the caller). That is, in the above example, if visit_type_FOO_fields() fails, we need to track that locally in order to clean up *obj. Meanwhile, the call to visit_end_implicit_struct() must be unconditional, whether or not we have detected earlier failure. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature