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