From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:45713) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1StOmv-0001av-JK for qemu-devel@nongnu.org; Mon, 23 Jul 2012 15:59:30 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1StOmu-0002j5-DB for qemu-devel@nongnu.org; Mon, 23 Jul 2012 15:59:29 -0400 Received: from mx1.redhat.com ([209.132.183.28]:48087) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1StOmu-0002iu-4z for qemu-devel@nongnu.org; Mon, 23 Jul 2012 15:59:28 -0400 Date: Mon, 23 Jul 2012 17:00:02 -0300 From: Luiz Capitulino Message-ID: <20120723170002.510fd7f0@doriath.home> In-Reply-To: <87pq7m1g4v.fsf@blackfin.pond.sub.org> References: <1342212261-19903-1-git-send-email-lcapitulino@redhat.com> <1342212261-19903-5-git-send-email-lcapitulino@redhat.com> <87fw8l1qkk.fsf@blackfin.pond.sub.org> <20120723131709.4780781e@doriath.home> <87pq7m1g4v.fsf@blackfin.pond.sub.org> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 4/8] qemu-option: add alias support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: jan.kiszka@siemens.com, aliguori@us.ibm.com, qemu-devel@nongnu.org, afaerber@suse.de On Mon, 23 Jul 2012 20:33:52 +0200 Markus Armbruster wrote: > Luiz Capitulino writes: > > > On Sat, 21 Jul 2012 10:11:39 +0200 > > Markus Armbruster wrote: > > > >> Luiz Capitulino writes: > >> > >> > Allow for specifying an alias for each option name, see next commits > >> > for examples. > >> > > >> > Signed-off-by: Luiz Capitulino > >> > --- > >> > qemu-option.c | 5 +++-- > >> > qemu-option.h | 1 + > >> > 2 files changed, 4 insertions(+), 2 deletions(-) > >> > > >> > diff --git a/qemu-option.c b/qemu-option.c > >> > index 65ba1cf..b2f9e21 100644 > >> > --- a/qemu-option.c > >> > +++ b/qemu-option.c > >> > @@ -623,7 +623,8 @@ static const QemuOptDesc *find_desc_by_name(const QemuOptDesc *desc, > >> > int i; > >> > > >> > for (i = 0; desc[i].name != NULL; i++) { > >> > - if (strcmp(desc[i].name, name) == 0) { > >> > + if (strcmp(desc[i].name, name) == 0 || > >> > + (desc[i].alias && strcmp(desc[i].alias, name) == 0)) { > >> > return &desc[i]; > >> > } > >> > } > >> > @@ -645,7 +646,7 @@ static void opt_set(QemuOpts *opts, const char *name, const char *value, > > static void opt_set(QemuOpts *opts, const char *name, const > bool prepend, Error **errp) > { > QemuOpt *opt; > const QemuOptDesc *desc; > Error *local_err = NULL; > > desc = find_desc_by_name(opts->list->desc, name); > if (!desc && !opts_accepts_any(opts)) { > error_set(errp, QERR_INVALID_PARAMETER, name); > return; > >> > } > >> > > >> > opt = g_malloc0(sizeof(*opt)); > >> > - opt->name = g_strdup(name); > >> > + opt->name = g_strdup(desc ? desc->name : name); > >> > opt->opts = opts; > >> > if (prepend) { > >> > QTAILQ_INSERT_HEAD(&opts->head, opt, next); > >> > >> Are you sure this hunk belongs to this patch? If yes, please explain > >> why :) > > > > Yes, I think it's fine because the change that makes it necessary to choose > > between desc->name and name is introduced by the hunk above. > > > > I think I now get why you have this hunk: > > We reach it only if the parameter with this name exists (desc non-null), > or opts accepts anthing (opts_accepts_any(opts). > > If it exists, name equals desc->name before your patch. But afterwards, > it could be either desc->name or desc->alias. You normalize to > desc->name. > > Else, all we can do is stick to name. > > Correct? Yes. > If yes, then "normal" options and "accept any" options behave > differently: the former set opts->name to the canonical name, even when > the user uses an alias. The latter set opts->name to whatever the user > uses. I got a bad feeling about that. Why? Or, more importantly, how do you think we should do it? For 'normal' options, the alias is just a different name to refer to the option name. At least in theory, it shouldn't matter that the option was set through the alias. For 'accept any' options, it's up to the code handling the options know what to do with it. It can also support aliases if it wants to, or just refuse it.