qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v9 12/37] qapi: Don't cast Enum* to int*


From: Markus Armbruster
Subject: Re: [Qemu-devel] [PATCH v9 12/37] qapi: Don't cast Enum* to int*
Date: Wed, 20 Jan 2016 19:08:29 +0100
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux)

Eric Blake <address@hidden> writes:

> C compilers are allowed to represent enums as a smaller type
> than int, if all enum values fit in the smaller type.  There
> are even compiler flags that force the use of this smaller
> representation, and using them changes the ABI of a binary.

Suggest "although using them".

> Therefore, our generated code for visit_type_ENUM() (for all
> qapi enums) was wrong for casting Enum* to int* when calling
> visit_type_enum().
>
> It appears that no one has been doing this for qemu, because
> if they had, we are potentially dereferencing beyond bounds
> or even risking a SIGBUS on platforms where unaligned pointer
> dereferencing is fatal.  Better is to avoid the practice
> entirely, and just use the correct types.
>
> This matches the fix for alternate qapi types, done earlier in
> commit 0426d53 "qapi: Simplify visiting of alternate types",
> with generated code changing as:
>
> | void visit_type_GuestDiskBusType(Visitor *v, GuestDiskBusType *obj, const 
> char *name, Error **errp)
> | {
> |-    visit_type_enum(v, (int *)obj, GuestDiskBusType_lookup, 
> "GuestDiskBusType", name, errp);
> |+    int tmp = *obj;
> |+    visit_type_enum(v, &tmp, GuestDiskBusType_lookup, "GuestDiskBusType", 
> name, errp);
> |+    *obj = tmp;
> | }

Long lines.  Do we have an example with a shorter enum name handy?

> Signed-off-by: Eric Blake <address@hidden>
> Reviewed-by: Marc-André Lureau <address@hidden>
>
> ---
> v9: mention earlier commit id, enhance commit message
> v8: no change
> v7: rebase on typo fix
> v6: new patch
> ---
>  scripts/qapi-visit.py | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
> index 4a4f67d..6bd188b 100644
> --- a/scripts/qapi-visit.py
> +++ b/scripts/qapi-visit.py
> @@ -178,12 +178,13 @@ 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)
>  {
> -    visit_type_enum(v, (int *)obj, %(c_name)s_lookup, "%(name)s", name, 
> errp);
> +    int tmp = *obj;
> +    visit_type_enum(v, &tmp, %(c_name)s_lookup, "%(name)s", name, errp);
> +    *obj = tmp;
>  }
>  ''',
>                   c_name=c_name(name), name=name)

Same pattern in qapi-visit-core.c, except we name the variable @value
there.  Your choice.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]