Re: [PATCH 13/46] qemu-option: Simplify around find_default_by_name()

[Vladimir Sementsov-Ogievskiy]

25.06.2020 16:12, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
>
> > On 6/24/20 11:43 AM, Markus Armbruster wrote:
> > > Signed-off-by: Markus Armbruster <armbru@redhat.com>
> > > ---
> > >    util/qemu-option.c | 13 ++++---------
> > >    1 file changed, 4 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/util/qemu-option.c b/util/qemu-option.c
> > > index ddcf3072c5..d9293814b4 100644
> > > --- a/util/qemu-option.c
> > > +++ b/util/qemu-option.c
> > > @@ -286,11 +286,9 @@ const char *qemu_opt_get(QemuOpts *opts, const char *name)
> > >        opt = qemu_opt_find(opts, name);
> > >        if (!opt) {
> > >            def_val = find_default_by_name(opts, name);
> > > -        if (def_val) {
> > > -            return def_val;
> > > -        }
> > > +        return def_val;
> > >        }
> > > -    return opt ? opt->str : NULL;
> > > +    return opt->str;
> > >    }
> >
> > You could go with even fewer lines and variables by inverting the logic:
> >
> > if (opt) {
> >      return opt->str;
> > }
> > return find_default_by_name(opts, name);
>
> Yes, that's better.
>
> > >      void qemu_opt_iter_init(QemuOptsIter *iter, QemuOpts *opts,
> > > const char *name)
> > > @@ -320,7 +318,7 @@ char *qemu_opt_get_del(QemuOpts *opts, const char *name)
> > >    {
> > >        QemuOpt *opt;
> > >        const char *def_val;
> > > -    char *str = NULL;
> > > +    char *str;
> > >          if (opts == NULL) {
> > >            return NULL;
> > > @@ -329,10 +327,7 @@ char *qemu_opt_get_del(QemuOpts *opts, const char *name)
> > >        opt = qemu_opt_find(opts, name);
> > >        if (!opt) {
> > >            def_val = find_default_by_name(opts, name);
> > > -        if (def_val) {
> > > -            str = g_strdup(def_val);
> > > -        }
> > > -        return str;
> > > +        return g_strdup(def_val);
> >
> > Similarly, you could drop def_val with:
> >   return g_strdup(find_default_by_name(opts, name));
>
> Your contracted version is still readable; sold.
>
> > Either way,
> > Reviewed-by: Eric Blake <eblake@redhat.com>

with suggested improvements:
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>



--
Best regards,
Vladimir