Eric Blake <ebl...@redhat.com> writes: > On 6/24/20 11:43 AM, Markus Armbruster wrote: >> Signed-off-by: Markus Armbruster <arm...@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 <ebl...@redhat.com> Thanks!