Eric Blake <ebl...@redhat.com> 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 <ebl...@redhat.com>
> Reviewed-by: Marc-André Lureau <marcandre.lur...@redhat.com>
>
> ---
> 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.
