[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] qapi: Fix crash with enum dealloc when kind is
From: |
Fam Zheng |
Subject: |
Re: [Qemu-devel] [PATCH] qapi: Fix crash with enum dealloc when kind is invalid |
Date: |
Thu, 11 Sep 2014 12:38:03 +0800 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
On Wed, 09/10 22:17, Eric Blake wrote:
> On 09/10/2014 06:53 PM, Fam Zheng wrote:
> > On Wed, 09/10 17:32, Paolo Bonzini wrote:
> >> Il 10/09/2014 17:02, Fam Zheng ha scritto:
> >>>> A bit hackish, but I don't have any better idea.
> >>>>
> >>>> Hmm... what about adding a new member to the visitors for "invalid enum"
> >>>> value? The dealloc visitor could override it to do nothing, while the
> >>>> default could abort or set an error. Would that work?
> >>>
> >>> The invalid state of enum still needs to be saved in the data. It is
> >>> detected
> >>> by the input visitor, but should be checked by other visitors (output,
> >>> dealloc)
> >>> later.
> >>
> >> Yes, that's fine. The only part where I'm not sure is the special
> >> casing of the _MAX enum.
> >>
> >
> > Yes, it is abusing. Let's add an _INVALID = 0 enum which is much clearer.
>
> If I understand correctly, you mean that for:
>
> { 'enum': 'Foo', 'data': [ 'one', 'two' ] }
>
> FOO_ONE would now be 1 instead of its current value of 0?
>
> We just barely saw a case where Hu Tao's code was relying on the
> implicit value 0 assigned to the first enum in the json file [1]
> although I strongly argued that it should be nuked (and so it was fixed
> in [2]). So I could live with reserving 0 for internal use for flagging
> parse errors (such as attempting to pass the string 'three' where a Foo
> value is expected).
>
> [1] https://lists.gnu.org/archive/html/qemu-devel/2014-09/msg01691.html
> [2] https://lists.gnu.org/archive/html/qemu-devel/2014-09/msg01938.html
>
A closer looking shows me a huge risk with _lookup table. We have for loops
starting with index 0 all over the place to peek info _lookup arrays, but in
this case it is _INVALID and shouldn't be looked at.
Maybe we have to put _INVALID at the end after _MAX.
Fam
- [Qemu-devel] [PATCH] qapi: Fix crash with enum dealloc when kind is invalid, Fam Zheng, 2014/09/10
- Re: [Qemu-devel] [PATCH] qapi: Fix crash with enum dealloc when kind is invalid, Paolo Bonzini, 2014/09/10
- Re: [Qemu-devel] [PATCH] qapi: Fix crash with enum dealloc when kind is invalid, Fam Zheng, 2014/09/10
- Re: [Qemu-devel] [PATCH] qapi: Fix crash with enum dealloc when kind is invalid, Paolo Bonzini, 2014/09/10
- Re: [Qemu-devel] [PATCH] qapi: Fix crash with enum dealloc when kind is invalid, Fam Zheng, 2014/09/10
- Re: [Qemu-devel] [PATCH] qapi: Fix crash with enum dealloc when kind is invalid, Eric Blake, 2014/09/11
- Re: [Qemu-devel] [PATCH] qapi: Fix crash with enum dealloc when kind is invalid,
Fam Zheng <=
- Re: [Qemu-devel] [PATCH] qapi: Fix crash with enum dealloc when kind is invalid, Michael Roth, 2014/09/11
- Re: [Qemu-devel] [PATCH] qapi: Fix crash with enum dealloc when kind is invalid, Paolo Bonzini, 2014/09/11
- Re: [Qemu-devel] [PATCH] qapi: Fix crash with enum dealloc when kind is invalid, Michael Roth, 2014/09/11
Re: [Qemu-devel] [PATCH] qapi: Fix crash with enum dealloc when kind is invalid, Michael Roth, 2014/09/10
[Qemu-devel] [PATCH] tests: add QMP input visitor test for unions with no discriminator, Michael Roth, 2014/09/10