[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 4/8] qemu-option: add alias support
From: |
Luiz Capitulino |
Subject: |
Re: [Qemu-devel] [PATCH 4/8] qemu-option: add alias support |
Date: |
Mon, 23 Jul 2012 17:00:02 -0300 |
On Mon, 23 Jul 2012 20:33:52 +0200
Markus Armbruster <address@hidden> wrote:
> Luiz Capitulino <address@hidden> writes:
>
> > On Sat, 21 Jul 2012 10:11:39 +0200
> > Markus Armbruster <address@hidden> wrote:
> >
> >> Luiz Capitulino <address@hidden> writes:
> >>
> >> > Allow for specifying an alias for each option name, see next commits
> >> > for examples.
> >> >
> >> > Signed-off-by: Luiz Capitulino <address@hidden>
> >> > ---
> >> > 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.
- Re: [Qemu-devel] [PATCH 1/8] qemu-option: qemu_opt_set_bool(): fix code duplication, (continued)
[Qemu-devel] [PATCH 2/8] qemu-option: opt_set(): split it up into more functions, Luiz Capitulino, 2012/07/13
[Qemu-devel] [PATCH 3/8] qemu-option: qemu_opts_validate(): fix duplicated code, Luiz Capitulino, 2012/07/13
[Qemu-devel] [PATCH 4/8] qemu-option: add alias support, Luiz Capitulino, 2012/07/13
- Re: [Qemu-devel] [PATCH 4/8] qemu-option: add alias support, Markus Armbruster, 2012/07/21
- Re: [Qemu-devel] [PATCH 4/8] qemu-option: add alias support, Luiz Capitulino, 2012/07/23
- Re: [Qemu-devel] [PATCH 4/8] qemu-option: add alias support, Markus Armbruster, 2012/07/23
- Re: [Qemu-devel] [PATCH 4/8] qemu-option: add alias support,
Luiz Capitulino <=
- Re: [Qemu-devel] [PATCH 4/8] qemu-option: add alias support, Markus Armbruster, 2012/07/24
- Re: [Qemu-devel] [PATCH 4/8] qemu-option: add alias support, Luiz Capitulino, 2012/07/24
- Re: [Qemu-devel] [PATCH 4/8] qemu-option: add alias support, Markus Armbruster, 2012/07/24
- Re: [Qemu-devel] [PATCH 4/8] qemu-option: add alias support, Luiz Capitulino, 2012/07/25
[Qemu-devel] [PATCH 5/8] machine: rename kernel_irqchip to kernel-irqchip, Luiz Capitulino, 2012/07/13
[Qemu-devel] [PATCH 6/8] machine: rename kvm_shadow_mem to kvm-shadow-mem, Luiz Capitulino, 2012/07/13
[Qemu-devel] [PATCH 7/8] machine: rename phandle_start to phandle-start, Luiz Capitulino, 2012/07/13
[Qemu-devel] [PATCH 8/8] machine: rename dt_compatible to dt-compatible, Luiz Capitulino, 2012/07/13