[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v11 22/28] qapi: Simplify visiting of alternate
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v11 22/28] qapi: Simplify visiting of alternate types |
Date: |
Mon, 16 Nov 2015 15:31:33 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Eric Blake <address@hidden> writes:
> On 11/10/2015 11:51 PM, Eric Blake wrote:
>> Previously, working with alternates required two lookup arrays
>> and some indirection: for type Foo, we created Foo_qtypes[]
>> which maps each qtype to a value of the generated FooKind enum,
>> then look up that value in FooKind_lookup[] like we do for other
>> union types.
>>
>> This has a couple of subtle bugs. First, the generator was
>> creating a call with a parameter '(int *) &(*obj)->type' where
>> type is an enum type; this is unsafe if the compiler chooses
>> to store the enum type in a different size than int, where
>> assigning through the wrong size pointer can corrupt data or
>> cause a SIGBUS.
>
> Yuck - we still have this bug for enums:
>
>> void visit_type_OnOffAuto(Visitor *v, OnOffAuto *obj, const char
>> *name, Error **errp)
>> {
>> visit_type_enum(v, (int *)obj, OnOffAuto_lookup, "OnOffAuto", name, errp);
>> }
>
> I guess I just added more work to my plate.
>
> But at the same time, no one has complained of the (int*)Enum causing
> SIGBUS on any of our platforms for either enums or alternates, so while
> we are doing gross things, I don't think we have to rush a fix into 2.5
> for this particular bug.
If it was a new bug, I'd push for fixing it in 2.5. Since it's ancient,
I think we should leave it alone in 2.5.
- [Qemu-devel] [PATCH v11 24/28] qapi: Add positive tests to qapi-schema-test, (continued)
- [Qemu-devel] [PATCH v11 22/28] qapi: Simplify visiting of alternate types, Eric Blake, 2015/11/11
- [Qemu-devel] [PATCH v11 28/28] qapi: Detect base class loops, Eric Blake, 2015/11/11
- [Qemu-devel] [PATCH v11 23/28] qapi: Fix alternates that accept 'number' but not 'int', Eric Blake, 2015/11/11
- [Qemu-devel] [PATCH v11 27/28] qapi: Move duplicate enum value checks to schema check(), Eric Blake, 2015/11/11
- [Qemu-devel] [PATCH v11 18/28] qerror: more error_setg() usage, Eric Blake, 2015/11/11