On 09/17/2014 03:32 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. > > However, we can generally rely on the fact that the always-present > .data void * field that we generate for these union types will > always be NULL in cases where .kind is uninitialized (at least, > there shouldn't be a reason where we'd do this purposefully). > > So pass this information on to Visitor implementation via these > optional start_union/end_union interfaces so this information > can be used to guard against the situation above. We will make > use of this information in a subsequent patch for the dealloc > visitor. > > Cc: qemu-sta...@nongnu.org > Reported-by: Fam Zheng <f...@redhat.com> > Suggested-by: Paolo Bonzini <pbonz...@redhat.com> > Reviewed-by: Paolo Bonzini <pbonz...@redhat.com> > Signed-off-by: Michael Roth <mdr...@linux.vnet.ibm.com> > ---
Reviewed-by: Eric Blake <ebl...@redhat.com> > > +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; > +} So we default to returning true (which implies safe to visit the union fields), and patch 2 creates the only case where this returns false (when data_present is false). I also note that errp is never set by this series, but it's fine to wire it up in anticipation of any future need. Took me a couple reads to get what's happening, but I agree with the results. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature