On Thu, Jul 05, 2018 at 08:02:21AM +0200, Markus Armbruster wrote:
> Peter Xu <peterx@redhat.com> writes:
>
> > When there is no parameter at all for a function prototype, return
> > "void" for the parameter list. This will happen after the next patch
> > where we removed the Error* for some of the emit functions.
>
> Error **, actually. Let's say
>
> qapi: Fix build_params() for empty parameter list
>
> build_params() returns '' instead of 'void' when there are no
> parameters. Can't happen now, but the next commit will change that.
Sure.
>
> >
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> > scripts/qapi/common.py | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
> > index 8b6708dbf1..105c82742f 100644
> > --- a/scripts/qapi/common.py
> > +++ b/scripts/qapi/common.py
> > @@ -1963,7 +1963,7 @@ extern const QEnumLookup %(c_name)s_lookup;
> > def build_params(arg_type, boxed, extra):
> > if not arg_type:
> > assert not boxed
> > - return extra
> > + return extra if extra else "void"
> > ret = ''
> > sep = ''
> > if boxed:
> > @@ -1980,7 +1980,7 @@ def build_params(arg_type, boxed, extra):
> > c_name(memb.name))
> > if extra:
> > ret += sep + extra
> > - return ret
> > + return ret if ret else "void"
> >
> >
> > #
>
> Please use ' instead of " for local consistency.
>
> Duplicating "if ret else 'void'" minimizes churn, but it's slightly
> ugly. I append my attempt to avoid it. What do you think?
>
>
> diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
> index 8b6708dbf1..01384c5360 100644
> --- a/scripts/qapi/common.py
> +++ b/scripts/qapi/common.py
> @@ -1961,15 +1961,13 @@ extern const QEnumLookup %(c_name)s_lookup;
>
>
> def build_params(arg_type, boxed, extra):
> - if not arg_type:
> - assert not boxed
> - return extra
> ret = ''
> sep = ''
> if boxed:
> + assert arg_type
> ret += '%s arg' % arg_type.c_param_type()
> sep = ', '
> - else:
> + elif arg_type:
> assert not arg_type.variants
> for memb in arg_type.members:
> ret += sep
> @@ -1980,7 +1978,7 @@ def build_params(arg_type, boxed, extra):
> c_name(memb.name))
> if extra:
> ret += sep + extra
> - return ret
> + return ret if ret else 'void'
Yeah, this looks good to me.
Thanks,
--
Peter Xu