Re: [Qemu-devel] [PATCH 1/3] qapi: add visit_start_union and visit_end_union

[Michael Roth <mdroth@linux.vnet.ibm.com>]

Quoting Eric Blake (2014-09-11 21:29:24)
> On 09/11/2014 05:20 PM, Michael Roth wrote:
> > In some cases an input visitor might bail out on filling out a
> > struct for various reasons, such as missing fields when running
> > in strict mode. In the case of a QAPI Union type, this may lead
> > to cases where the .kind field which encodes the union type
> > is uninitialized. Subsequently, other visitors, such as the
> > dealloc visitor, may use this .kind value as if it were
> > initialized, leading to assumptions about the union type which
> > in this case may lead to segfaults. For example, freeing an
> > integer value.
> > 
> 
> >  
> > +bool visit_start_union(Visitor *v, bool data_present, Error **errp)
> > +{
> > +    if (v->start_union) {
> > +        return v->start_union(v, data_present, errp);
> > +    }
> > +    return true;
> > +}
> 
> Any rules on whether errp must be set if returning false, and must not
> be set if returning true?  If so, do we need a bool return, or is errp
> sufficient?

No restrictions in that regard, since this just an indicator of whether
any work should be done on the union fields. For instance the dealloc
visitor should continue operating normally and just skip over these
rather than going into an error state.

> 
> 
> > +++ b/scripts/qapi-visit.py
> > @@ -357,6 +357,9 @@ void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error **e
> >          if (err) {
> >              goto out_obj;
> >          }
> > +        if (!visit_start_union(m, !!(*obj)->data, &err)) {
> > +            goto out_obj;
> > +        }
> >          switch ((*obj)->kind) {
> 
> and if there aren't rules, then a visitor that sets err but still
> returns true would result in this code not exiting early, but passing an
> already-set error into the switch, which is probably not desirable.

Ahh, good catch. We should be jumping to out_obj: on error as well.

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