Eric Blake <ebl...@redhat.com> writes:
> Since we have consolidated all generated code to use 'err' as
> the name of the local variable for error detection, we can
> simplify the decision on whether to skip error detection (useful
> for deallocation paths) to be a boolean.
>
> This requires the python 2.5 ternary syntax.
Let's drop this sentence.
> Signed-off-by: Eric Blake <ebl...@redhat.com>
>
> ---
> v7: new patch; rfc as it depends on Markus' configure change to
> require python 2.6
> ---
> scripts/qapi-commands.py | 4 +---
> scripts/qapi.py | 23 ++++++++++-------------
> 2 files changed, 11 insertions(+), 16 deletions(-)
>
> diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
> index 9d214a6..43a893b 100644
> --- a/scripts/qapi-commands.py
> +++ b/scripts/qapi-commands.py
> @@ -101,19 +101,17 @@ def gen_marshal_input_visit(arg_type, dealloc=False):
> return ret
>
> if dealloc:
> - errarg = None
> ret += mcgen('''
> qmp_input_visitor_cleanup(qiv);
> qdv = qapi_dealloc_visitor_new();
> v = qapi_dealloc_get_visitor(qdv);
> ''')
> else:
> - errarg = 'err'
> ret += mcgen('''
> v = qmp_input_get_visitor(qiv);
> ''')
>
> - ret += gen_visit_fields(arg_type.members, errarg=errarg)
> + ret += gen_visit_fields(arg_type.members, skiperr=dealloc)
>
> if dealloc:
> ret += mcgen('''
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index ada6380..7761b4b 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -1537,23 +1537,19 @@ def gen_params(arg_type, extra):
> return ret
>
>
> -def gen_err_check(err='err', label='out'):
> - if not err:
> +def gen_err_check(label='out', skiperr=False):
> + if skiperr:
> return ''
> return mcgen('''
> - if (%(err)s) {
> + if (err) {
> goto %(label)s;
> }
> ''',
> - err=err, label=label)
> + label=label)
>
>
> -def gen_visit_fields(members, prefix='', need_cast=False, errarg='err'):
> +def gen_visit_fields(members, prefix='', need_cast=False, skiperr=False):
> ret = ''
> - if errarg:
> - errparg = '&' + errarg
> - else:
> - errparg = 'NULL'
>
> for memb in members:
> if memb.optional:
> @@ -1561,8 +1557,9 @@ def gen_visit_fields(members, prefix='',
> need_cast=False, errarg='err'):
> visit_optional(v, &%(prefix)shas_%(c_name)s, "%(name)s", %(errp)s);
> ''',
> prefix=prefix, c_name=c_name(memb.name),
> - name=memb.name, errp=errparg)
> - ret += gen_err_check(err=errarg)
> + name=memb.name,
> + errp='&err' if not skiperr else 'NULL')
> + ret += gen_err_check(skiperr=skiperr)
> ret += mcgen('''
> if (%(prefix)shas_%(c_name)s) {
> ''',
> @@ -1580,8 +1577,8 @@ def gen_visit_fields(members, prefix='',
> need_cast=False, errarg='err'):
> ''',
> c_type=memb.type.c_name(), prefix=prefix, cast=cast,
> c_name=c_name(memb.name), name=memb.name,
> - errp=errparg)
> - ret += gen_err_check(err=errarg)
> + errp='&err' if not skiperr else 'NULL')
> + ret += gen_err_check(skiperr=skiperr)
>
> if memb.optional:
> pop_indent()
I'm not afraid of ternaries, but I guess I would've gone for the minimal
change here, i.e. something like:
-def gen_visit_fields(members, prefix='', need_cast=False, errarg='err'):
+def gen_visit_fields(members, prefix='', need_cast=False, skiperr=False):
ret = ''
- if errarg:
- errparg = '&' + errarg
- else:
+ if skiperr:
errparg = 'NULL'
+ else:
+ errparg = '&err'
for memb in members:
if memb.optional:
@@ -1562,7 +1562,7 @@ def gen_visit_fields(members, prefix='', need_cast=False,
errarg='err'):
''',
prefix=prefix, c_name=c_name(memb.name),
name=memb.name, errp=errparg)
- ret += gen_err_check(err=errarg)
+ ret += gen_err_check(skiperr=skiperr)
ret += mcgen('''
if (%(prefix)shas_%(c_name)s) {
''',
@@ -1581,7 +1581,7 @@ def gen_visit_fields(members, prefix='', need_cast=False,
errarg='err'):
c_type=memb.type.c_name(), prefix=prefix, cast=cast,
c_name=c_name(memb.name), name=memb.name,
errp=errparg)
- ret += gen_err_check(err=errarg)
+ ret += gen_err_check(skiperr=skiperr)
if memb.optional:
pop_indent()
Matter of taste.
