On 01/05/2016 10:22 AM, Marc-André Lureau wrote: > Hi > > On Mon, Dec 21, 2015 at 6:08 PM, Eric Blake <ebl...@redhat.com> wrote: >> Returning a partial object on error is an invitation for a careless >> caller to leak memory. As no one outside the testsuite was actually >> relying on these semantics, it is cleaner to just document and >> guarantee that ALL pointer-based visit_type_FOO() functions always >> leave a safe value in *obj during an input visitor (either the new >> object on success, or NULL if an error is encountered). >> >> Since input visitors have blind assignment semantics, we have to >> track the result of whether an assignment is made all the way down >> to each visitor callback implementation, to avoid making decisions >> based on potentially uninitialized storage. >> >> Note that we still leave *obj unchanged after a scalar-based >> visit_type_FOO(); I did not feel like auditing all uses of >> visit_type_Enum() to see if the callers would tolerate a specific >> sentinel value (not to mention having to decide whether it would >> be better to use 0 or ENUM__MAX as that sentinel). >> >> Signed-off-by: Eric Blake <ebl...@redhat.com> >> > > nice cleanup, few issues: >
>> @@ -237,6 +254,10 @@ void visit_type_str(Visitor *v, const char *name, char
>> **obj, Error **errp)
>> }
>> */
>> v->type_str(v, name, obj, errp);
>> + if (!visit_is_output(v) && err) {
>> + *obj = NULL;
>> + }
>
> This will always evelatute to false, you need to change the line above I
> suppose
>
>> + error_propagate(errp, err);
Oh right, that needs to be v->type_str(..., &err).
I'll have to double-check that no assertions trigger with the fixed
code, and provide the fixup patch. I don't know if Markus will want me
to spin a v9, but I'll wait for his comments before deciding if a full
respin is needed.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
