[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v9 35/37] qapi: Change visit_type_FOO() to no lo
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v9 35/37] qapi: Change visit_type_FOO() to no longer return partial objects |
Date: |
Fri, 29 Jan 2016 08:13:22 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.0 |
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
[Qemu-devel] [PATCH v9 34/37] qapi: Simplify semantics of visit_next_list(), Eric Blake, 2016/01/19
[Qemu-devel] [PATCH v9 15/37] qom: Swap 'name' next to visitor in ObjectPropertyAccessor, Eric Blake, 2016/01/19
[Qemu-devel] [PATCH v9 14/37] qapi: Swap visit_* arguments for consistent 'name' placement, Eric Blake, 2016/01/19