diff mbox

[v10,03/25] qapi: More robust conditions for when labels are needed

Message ID 1445576998-2921-4-git-send-email-eblake@redhat.com
State New
Headers show

Commit Message

Eric Blake Oct. 23, 2015, 5:09 a.m. UTC
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 <eblake@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(-)

Comments

Markus Armbruster Oct. 23, 2015, 12:44 p.m. UTC | #1
Eric Blake <eblake@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 <eblake@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.
diff mbox

Patch

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:
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: