Luiz Capitulino <lcapitul...@redhat.com> writes:
> On Sat, 21 Jul 2012 10:11:39 +0200
> Markus Armbruster <arm...@redhat.com> wrote:
>
>> Luiz Capitulino <lcapitul...@redhat.com> writes:
>>
>> > Allow for specifying an alias for each option name, see next commits
>> > for examples.
>> >
>> > Signed-off-by: Luiz Capitulino <lcapitul...@redhat.com>
>> > ---
>> > 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?
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.
> Of course that it's possible to move this to a separate patch, but I don't
> think it's worth it, as name is always valid with the current code.
>
>> > diff --git a/qemu-option.h b/qemu-option.h
>> > index 951dec3..7106d2f 100644
>> > --- a/qemu-option.h
>> > +++ b/qemu-option.h
>> > @@ -94,6 +94,7 @@ enum QemuOptType {
>> >
>> > typedef struct QemuOptDesc {
>> > const char *name;
>> > + const char *alias;
>> > enum QemuOptType type;
>> > const char *help;
>> > } QemuOptDesc;
>>
