From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:35550) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UZKLh-0007je-43 for qemu-devel@nongnu.org; Mon, 06 May 2013 08:16:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UZKLf-0005zH-JI for qemu-devel@nongnu.org; Mon, 06 May 2013 08:16:57 -0400 Received: from mx1.redhat.com ([209.132.183.28]:62987) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UZKLf-0005yy-Bg for qemu-devel@nongnu.org; Mon, 06 May 2013 08:16:55 -0400 From: Markus Armbruster References: <1365575111-4476-1-git-send-email-wdongxu@linux.vnet.ibm.com> <1365575111-4476-2-git-send-email-wdongxu@linux.vnet.ibm.com> Date: Mon, 06 May 2013 14:16:06 +0200 In-Reply-To: <1365575111-4476-2-git-send-email-wdongxu@linux.vnet.ibm.com> (Dong Xu Wang's message of "Wed, 10 Apr 2013 14:25:06 +0800") Message-ID: <87vc6wl2wp.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH V13 1/6] add def_value_str in QemuOptDesc struct and rewrite qemu_opts_print List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Dong Xu Wang Cc: kwolf@redhat.com, wdongxu@cn.ibm.com, qemu-devel@nongnu.org, stefanha@redhat.com Dong Xu Wang writes: > qemu_opts_print has no user now, so can re-write the function safely. > > qemu_opts_print will be used while using "qemu-img create", it will > produce the same output as previous code. > > The behavior of this function has changed: > > 1. Print every possible option, whether a value has been set or not. > 2. Option descriptors may provide a default value. > 3. Print to stdout instead of stderr. > > Previously the behavior was to print every option that has been set. > Options that have not been set would be skipped. > > Signed-off-by: Dong Xu Wang > --- > v12->v13 > 1) re-write commit message. > > v11->v12 > 1) make def_value_str become the real default value string in opt_set > function. > > v10->v11: > 1) print all values that have actually been assigned while accept-any > cases. > > v7->v8: > 1) print "elements => accept any params" while opts_accepts_any() == > true. > 2) since def_print_str is the default value if an option isn't set, > so rename it to def_value_str. > > > include/qemu/option.h | 3 ++- > util/qemu-option.c | 33 +++++++++++++++++++++++++++------ > 2 files changed, 29 insertions(+), 7 deletions(-) > > diff --git a/include/qemu/option.h b/include/qemu/option.h > index bdb6d21..b928ab0 100644 > --- a/include/qemu/option.h > +++ b/include/qemu/option.h > @@ -96,6 +96,7 @@ typedef struct QemuOptDesc { > const char *name; > enum QemuOptType type; > const char *help; > + const char *def_value_str; > } QemuOptDesc; > > struct QemuOptsList { > @@ -152,7 +153,7 @@ QDict *qemu_opts_to_qdict(QemuOpts *opts, QDict *qdict); > void qemu_opts_absorb_qdict(QemuOpts *opts, QDict *qdict, Error **errp); > > typedef int (*qemu_opts_loopfunc)(QemuOpts *opts, void *opaque); > -int qemu_opts_print(QemuOpts *opts, void *dummy); > +int qemu_opts_print(QemuOpts *opts); > int qemu_opts_foreach(QemuOptsList *list, qemu_opts_loopfunc func, void *opaque, > int abort_on_failure); > > diff --git a/util/qemu-option.c b/util/qemu-option.c > index 8b74bf1..57cdd57 100644 > --- a/util/qemu-option.c > +++ b/util/qemu-option.c > @@ -646,6 +646,7 @@ static void opt_set(QemuOpts *opts, const char *name, const char *value, > } > opt->desc = desc; > opt->str = g_strdup(value); > + opt->str = g_strdup(value ?: desc->def_value_str); Memory leak. Did you forget to delete the previous line? You do it in PATCH 4/6, plugging the leak... > qemu_opt_parse(opt, &local_err); > if (error_is_set(&local_err)) { > error_propagate(errp, local_err); opt_set() now accepts a null value argument, and so do its callers qemu_opt_set(), qemu_opt_set_err(), qemu_opts_set(). Why? > @@ -860,16 +861,36 @@ void qemu_opts_del(QemuOpts *opts) > g_free(opts); > } > > -int qemu_opts_print(QemuOpts *opts, void *dummy) > +int qemu_opts_print(QemuOpts *opts) > { > QemuOpt *opt; > + QemuOptDesc *desc = opts->list->desc; > > - fprintf(stderr, "%s: %s:", opts->list->name, > - opts->id ? opts->id : ""); > - QTAILQ_FOREACH(opt, &opts->head, next) { > - fprintf(stderr, " %s=\"%s\"", opt->name, opt->str); > + if (desc[0].name == NULL) { > + QTAILQ_FOREACH(opt, &opts->head, next) { > + printf("%s=\"%s\" ", opt->name, opt->str); > + } > + return 0; > + } > + for (; desc && desc->name; desc++) { > + const char *value = desc->def_value_str; > + QemuOpt *opt; > + > + opt = qemu_opt_find(opts, desc->name); > + if (opt) { > + value = opt->str; > + } > + > + if (!value) { > + continue; > + } > + > + if (desc->type == QEMU_OPT_STRING) { > + printf("%s='%s' ", desc->name, value); > + } else { > + printf("%s=%s ", desc->name, value); > + } > } > - fprintf(stderr, "\n"); > return 0; > }