diff mbox

[v12,31/36] qapi: Simplify visiting of alternate types

Message ID 87wptee7ud.fsf@blackfin.pond.sub.org
State New
Headers show

Commit Message

Markus Armbruster Nov. 19, 2015, 8:01 a.m. UTC 
Eric Blake <eblake@redhat.com> writes:

> On 11/18/2015 11:46 AM, Markus Armbruster wrote:
>> Eric Blake <eblake@redhat.com> 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

Comments

Eric Blake Nov. 19, 2015, 2:08 p.m. UTC | #1 
On 11/19/2015 01:01 AM, Markus Armbruster wrote:

>>>> 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('''

That works for me.
diff mbox

Patch

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)