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

[Eric Blake]

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: address@hidden
> Reported-by: Fam Zheng <address@hidden>
> Suggested-by: Paolo Bonzini <address@hidden>
> Reviewed-by: Paolo Bonzini <address@hidden>
> Signed-off-by: Michael Roth <address@hidden>
> ---

Reviewed-by: Eric Blake <address@hidden>

>  
> +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