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

[Peter Krempa]

On 10/30/13 05:19, Chen Hanxiao wrote:
> From: Chen Hanxiao <chenhanxiao at cn.fujitsu.com>
> 
> 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 at cn.fujitsu.com>
> ---
>  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


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 901 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20131030/47872093/attachment-0001.sig>