Re: [Qemu-devel] [PATCH v8 34/35] qapi: Change visit_type_FOO() to no longer return partial objects

[Eric Blake]

On 01/05/2016 10:22 AM, Marc-André Lureau wrote:
> Hi
> 
> On Mon, Dec 21, 2015 at 6:08 PM, Eric Blake <address@hidden> 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 <address@hidden>
>>
> 
> 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