[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/3] qapi: add visit_start_union and visit_end_u
From: |
Michael Roth |
Subject: |
Re: [Qemu-devel] [PATCH 1/3] qapi: add visit_start_union and visit_end_union |
Date: |
Fri, 12 Sep 2014 10:22:23 -0500 |
User-agent: |
alot/0.3.4 |
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
- [Qemu-devel] [PATCH 0/3] qapi: fix crash in dealloc visitor for union types, Michael Roth, 2014/09/11
- [Qemu-devel] [PATCH 2/3] qapi: dealloc visitor, implement visit_start_union, Michael Roth, 2014/09/11
- [Qemu-devel] [PATCH 3/3] tests: add QMP input visitor test for unions with no discriminator, Michael Roth, 2014/09/11
- [Qemu-devel] [PATCH 1/3] qapi: add visit_start_union and visit_end_union, Michael Roth, 2014/09/11
- Re: [Qemu-devel] [PATCH 1/3] qapi: add visit_start_union and visit_end_union, Eric Blake, 2014/09/11
- Re: [Qemu-devel] [PATCH 1/3] qapi: add visit_start_union and visit_end_union,
Michael Roth <=
- Re: [Qemu-devel] [PATCH 1/3] qapi: add visit_start_union and visit_end_union, Paolo Bonzini, 2014/09/12
- Re: [Qemu-devel] [PATCH 1/3] qapi: add visit_start_union and visit_end_union, Michael Roth, 2014/09/12
- Re: [Qemu-devel] [PATCH 1/3] qapi: add visit_start_union and visit_end_union, Paolo Bonzini, 2014/09/12
- Re: [Qemu-devel] [PATCH 1/3] qapi: add visit_start_union and visit_end_union, Michael Roth, 2014/09/12
- Re: [Qemu-devel] [PATCH 1/3] qapi: add visit_start_union and visit_end_union, Paolo Bonzini, 2014/09/12
- Re: [Qemu-devel] [PATCH 1/3] qapi: add visit_start_union and visit_end_union, Michael Roth, 2014/09/12
[Qemu-devel] [PATCH 4/3] qemu-iotests: Test missing "driver" key for blockdev-add, Fam Zheng, 2014/09/11
Re: [Qemu-devel] [PATCH 0/3] qapi: fix crash in dealloc visitor for union types, Fam Zheng, 2014/09/11