Eric Blake <ebl...@redhat.com> 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 <ebl...@redhat.com>
>
> ---
> 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.
