Re: [Qemu-devel] [PATCH v12 31/36] qapi: Simplify visiting of alternate types

[Markus Armbruster]

Eric Blake <address@hidden> writes:

> On 11/18/2015 11:46 AM, Markus Armbruster wrote:
>> Eric Blake <address@hidden> writes:
>> 
>>> 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.  [We still have the casting bug for our enum
>>> visitors, but that's a topic for a different patch.]
>> 
>> I'm not sure I get the last sentence.
>
> I was referring to our casts of enum types to int* inside visit_type_Enum():
> https://lists.gnu.org/archive/html/qemu-devel/2015-11/msg03408.html
>
> If you have a better wording for it, or want to drop the parenthetical
> altogether, I'm fine.

What about this: replace the parenthetical with

    Related bug, not fixed in this patch: qapi-visit.py's
    gen_visit_enum() generates a cast of its enum * argument to int *.
    Marked FIXME.

and squash in

diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index e8b53b3..4797d6e 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -172,6 +172,7 @@ out:
 
 
 def gen_visit_enum(name):
+    # FIXME cast from enum *obj to int * invalidly assumes enum is int
     return mcgen('''
 
 void visit_type_%(c_name)s(Visitor *v, %(c_name)s *obj, const char *name, 
Error **errp)