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

[Eric Blake]

On 01/29/2016 05:03 AM, Markus Armbruster wrote:

> 
> With (1) don't assign, the caller can pick an error value by assigning
> it before the visit, and it must not access the value on error unless it
> does.
> 
> With (2) assign zero, the caller can't pick an error value, but may
> safely access the value even on error.
> 
> Tradeoff.  I figure either can work for us.
> 
>>> (3) Assign null pointer, else don't assign anything
>>>
>>>     CON: inconsistent
>>>     CON: mix of (1)'s and (2)'s CON
>>
>> Which I think is what I did in this patch.
> 
> I don't like the inconsistency.  It complicates the interface.

I'll go ahead and audit to see whether more scalar visits were relying
on (1) and would have to be rewritten to use style (2); vs. whether more
pointer visits were passing in uninitialized obj and would have to be
rewritten to use style (1).

> I think behavior (1) don't assign and (2) assign zero both work, we just
> have to pick one and run with it.
> 
> If we pick behavior (1) don't assign, then we should assert something
> like !obj || !*obj on entry.  With such assertions in place, I think (1)
> should be roughly as safe as (2).

I think your assessment is right, and it's now just a matter of seeing
which way to get to a consistent state is less effort (I may still end
up doing both ways as competing patches, for comparison purposes).


> or maybe returns whether something was allocated:
> 
>     out_obj:
>         if (visit_end_struct(v) && err) {
>            qapi_free_T(*obj);
>         }

I'm liking that.  Dealloc and output visitors always return false, and
input visitors may need to track something on their stack for whether
they allocated or returned error earlier on, but it results in less
generated output.  Basically, it's lowering the 'bool allocated' that I
added in this attempt out of the generated code and into the visitors.

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

signature.asc
Description: OpenPGP digital signature