qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH v10 03/25] qapi: More robust conditions for when


From: Markus Armbruster
Subject: Re: [Qemu-devel] [PATCH v10 03/25] qapi: More robust conditions for when labels are needed
Date: Fri, 23 Oct 2015 14:44:29 +0200
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux)

Eric Blake <address@hidden> writes:

> We were using regular expressions to see if ret included
> any earlier text that emitted a 'goto out;' line, to decide
> whether we needed to output an 'out:' label.  But this is
> fragile, if the ret text can possibly combine more than one
> generated function body, where the first function used a
> goto but the second does not.  Change the code to just check
> for the known conditions which cause an error check to be
> needed.  Besides, it's slightly more efficient to use plain
> checks than regular expression searching.
>
> No change to generated code.
>
> Signed-off-by: Eric Blake <address@hidden>
>
> ---
> v10: new patch, split in part from 6/17
> ---
>  scripts/qapi-commands.py | 2 +-
>  scripts/qapi-visit.py    | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
> index 43a893b..d6a4bf4 100644
> --- a/scripts/qapi-commands.py
> +++ b/scripts/qapi-commands.py
> @@ -175,7 +175,7 @@ def gen_marshal(name, arg_type, ret_type):
>      ret += gen_marshal_input_visit(arg_type)
>      ret += gen_call(name, arg_type, ret_type)
>
> -    if re.search('^ *goto out;', ret, re.MULTILINE):
> +    if (arg_type and arg_type.members) or ret_type:
>          ret += mcgen('''
>
>  out:

I think this could use a comment pointing to the spot that generates the
goto out.  I believe it's exactly gen_err_check() in gen_visit_fields().

Now let me convince myself your condition works.  We call
gen_visit_fields() when arg_type, and gen_visit_fields() calls
gen_err_check() for each memb in arg_type.members.  Good.

> diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
> index d0759d7..e878018 100644
> --- a/scripts/qapi-visit.py
> +++ b/scripts/qapi-visit.py
> @@ -87,7 +87,7 @@ static void visit_type_%(c_name)s_fields(Visitor *v, 
> %(c_name)s **obj, Error **e
>
>      ret += gen_visit_fields(members, prefix='(*obj)->')
>
> -    if re.search('^ *goto out;', ret, re.MULTILINE):
> +    if base or members:
>          ret += mcgen('''
>
>  out:

Again, could use a comment.  I believe it's gen_err_check() here, and in
gen_visit_fields().

Now let me convince myself your condition works.  We call
gen_err_check() when base, and gen_visit_fields() unconditionally, which
in turn calls gen_err_check() for each memb in members.  Good.



reply via email to

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