[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.
[Qemu-devel] [PATCH v10 02/25] qapi: More idiomatic string operations, Eric Blake, 2015/10/23
[Qemu-devel] [PATCH v10 03/25] qapi: More robust conditions for when labels are needed, Eric Blake, 2015/10/23
- Re: [Qemu-devel] [PATCH v10 03/25] qapi: More robust conditions for when labels are needed,
Markus Armbruster <=
[Qemu-devel] [PATCH v10 06/25] vnc: Hoist allocation of VncBasicInfo to callers, Eric Blake, 2015/10/23
[Qemu-devel] [PATCH v10 08/25] qapi-types: Refactor base fields output, Eric Blake, 2015/10/23
[Qemu-devel] [PATCH v10 01/25] tests/qapi-schema: Test for reserved names, empty struct, Eric Blake, 2015/10/23
[Qemu-devel] [PATCH v10 04/25] qapi: Reserve '*List' type names for list types, Eric Blake, 2015/10/23