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

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1622 bytes --]

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?


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

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]