Re: [PATCH]virsh: track alias option and improve error message when option duplicate its alias

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10/30/13 05:19, Chen Hanxiao wrote:
> From: Chen Hanxiao <chenhanxiao@xxxxxxxxxxxxxx>
> 
> commit 2b172a8effa712aee97a21a64d2d02060958f9b2 allow
> alias to expand to opt=value pair.
> That means alias may not look alike since then.
> 
> With this patch we will also track alias.
> If we type command with one option and another marked
> as its alias, we will get an error message like:
> error: option '--AA' duplicate its alias '--AAA'

s/duplicate/duplicates/ ... to match the error down below.


> 
> Signed-off-by: Chen Hanxiao <chenhanxiao@xxxxxxxxxxxxxx>
> ---
>  tools/virsh.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/virsh.c b/tools/virsh.c
> index bad78c9..423e2d8 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -1101,11 +1101,18 @@ vshCmddefGetOption(vshControl *ctl, const vshCmdDef *cmd, const char *name,
>                      if (VIR_STRDUP(*optstr, value + 1) < 0)
>                          goto cleanup;
>                  }
> +                *opts_seen |= 1 << i;
>                  continue;
>              }
>              if ((*opts_seen & (1 << i)) && opt->type != VSH_OT_ARGV) {
> -                vshError(ctl, _("option --%s already seen"), name);
> -                goto cleanup;
> +                if ((*opts_seen & (1 << (i - 1)))) {
> +                    vshError(ctl, _("option '--%s' duplicates its alias '--%s'"),
> +                             cmd->opts[i].name, cmd->opts[i-1].name);

This is not right. This code depends on the aliased option being right
before the target option in the array describing the options. If you
move the options in the array around you may get strange error messages.

> +                    goto cleanup;
> +                } else {
> +                    vshError(ctl, _("option '--%s' already seen"), name);
> +                    goto cleanup;
> +                }
>              }
>              *opts_seen |= 1 << i;
>              *opt_index = i;
> 

The idea is good, but you need to make sure that the code doesn't depend
on ordering of the options in the array.

Peter


Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]