Re: [Qemu-devel] [PULL v2 16/20] qapi: Consistent generated code: prefer common indentation

[Marc-André Lureau]

Hi,

This patch introduces a regression:

$ x86_64-softmmu/qemu-system-x86_64 -netdev ?

Program received signal SIGSEGV, Segmentation fault.
0x0000555555a0bc0f in visit_type_NetClientOptions (v=0x5555564e19e0,
obj=0x555556510438, name=0x555555af7041 "opts", errp=0x7fffffffd450)
at qapi-visit.c:6906
6906        visit_end_union(v, !!(*obj)->data, &err);
(gdb) print *obj
$1 = (NetClientOptions *) 0x0

On Mon, Oct 12, 2015 at 8:19 PM, Markus Armbruster <address@hidden> wrote:
> From: Eric Blake <address@hidden>
>
> We had some pointless differences in the generated code for visit,
> command marshalling, and events; unifying them makes it easier for
> future patches to consolidate to common helper functions.
> This is one patch of a series to clean up these differences.
>
> This patch adjusts gen_visit_union() to use the same indentation
> as other functions, namely, by jumping early to the error label
> if the object was not set rather than placing the rest of the
> body inside an if for when it is set.
>
> No change in semantics to the generated code.
>
> Signed-off-by: Eric Blake <address@hidden>
> Message-Id: <address@hidden>
> Signed-off-by: Markus Armbruster <address@hidden>
> ---
>  scripts/qapi-visit.py | 53 
> ++++++++++++++++++++++++++-------------------------
>  1 file changed, 27 insertions(+), 26 deletions(-)
>
> diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
> index 5a453ea..fe9780e 100644
> --- a/scripts/qapi-visit.py
> +++ b/scripts/qapi-visit.py
> @@ -264,16 +264,18 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s 
> **obj, const char *name, Error
>      if (err) {
>          goto out;
>      }
> -    if (*obj) {
> +    if (!*obj) {
> +        goto out_obj;
> +    }
>  ''',
>                   c_name=c_name(name), name=name)
>
>      if base:
>          ret += mcgen('''
> -        visit_type_%(c_name)s_fields(v, obj, &err);
> -        if (err) {
> -            goto out_obj;
> -        }
> +    visit_type_%(c_name)s_fields(v, obj, &err);
> +    if (err) {
> +        goto out_obj;
> +    }
>  ''',
>                       c_name=c_name(name))
>
> @@ -282,14 +284,14 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s 
> **obj, const char *name, Error
>          # we pointlessly use a different key for simple unions
>          tag_key = 'type'
>      ret += mcgen('''
> -        visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, "%(name)s", &err);
> -        if (err) {
> -            goto out_obj;
> -        }
> -        if (!visit_start_union(v, !!(*obj)->data, &err) || err) {
> -            goto out_obj;
> -        }
> -        switch ((*obj)->%(c_name)s) {
> +    visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, "%(name)s", &err);
> +    if (err) {
> +        goto out_obj;
> +    }
> +    if (!visit_start_union(v, !!(*obj)->data, &err) || err) {
> +        goto out_obj;
> +    }
> +    switch ((*obj)->%(c_name)s) {
>  ''',
>                   c_type=variants.tag_member.type.c_name(),
>                   # TODO ugly special case for simple union
> @@ -302,37 +304,36 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s 
> **obj, const char *name, Error
>          # TODO ugly special case for simple union
>          simple_union_type = var.simple_union_type()
>          ret += mcgen('''
> -        case %(case)s:
> +    case %(case)s:
>  ''',
>                       case=c_enum_const(variants.tag_member.type.name,
>                                         var.name))
>          if simple_union_type:
>              ret += mcgen('''
> -            visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, "data", &err);
> +        visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, "data", &err);
>  ''',
>                           c_type=simple_union_type.c_name(),
>                           c_name=c_name(var.name))
>          else:
>              ret += mcgen('''
> -            visit_type_implicit_%(c_type)s(v, &(*obj)->%(c_name)s, &err);
> +        visit_type_implicit_%(c_type)s(v, &(*obj)->%(c_name)s, &err);
>  ''',
>                           c_type=var.type.c_name(),
>                           c_name=c_name(var.name))
>          ret += mcgen('''
> -            break;
> +        break;
>  ''')
>
>      ret += mcgen('''
> -        default:
> -            abort();
> -        }
> -out_obj:
> -        error_propagate(errp, err);
> -        err = NULL;
> -        visit_end_union(v, !!(*obj)->data, &err);
> -        error_propagate(errp, err);
> -        err = NULL;
> +    default:
> +        abort();
>      }
> +out_obj:
> +    error_propagate(errp, err);
> +    err = NULL;
> +    visit_end_union(v, !!(*obj)->data, &err);
> +    error_propagate(errp, err);
> +    err = NULL;
>      visit_end_struct(v, &err);
>  out:
>      error_propagate(errp, err);
> --
> 2.4.3
>
>



-- 
Marc-André Lureau