[libvirt] [PATCH v2 03/12] virsh: Conditionally Ignore the first entry in list of completions

[Michal Privoznik]

On 05/08/2018 04:20 PM, Lin Ma wrote:
> The first entry in the returned array is the substitution for TEXT. It
> causes unnecessary output if other commands or options share the same
> prefix, e.g.
> 
> $ virsh des<TAB><TAB>
> des      desc     destroy
> 
> or
> 
> $ virsh domblklist --d<TAB><TAB>
> --d        --details  --domain
> 
> This patch fixes the above issue.
> 
> Signed-off-by: Lin Ma <lma at suse.com>
> ---
>  tools/vsh.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/vsh.c b/tools/vsh.c
> index 73ec007e56..57f7589b53 100644
> --- a/tools/vsh.c
> +++ b/tools/vsh.c
> @@ -3458,6 +3458,7 @@ cmdComplete(vshControl *ctl, const vshCmd *cmd)
>      const vshCmdOpt *opt = NULL;
>      char **matches = NULL, **iter;
>      virBuffer buf = VIR_BUFFER_INITIALIZER;
> +    int n;

This needs to be size_t. Even though it's not used to directly access
entries of @matches array, it kind of is. It's used to count them. And
int is not guaranteed to be able to address all of them.

>  
>      if (vshCommandOptStringQuiet(ctl, cmd, "string", &arg) <= 0)
>          goto cleanup;
> @@ -3493,8 +3494,11 @@ cmdComplete(vshControl *ctl, const vshCmd *cmd)
>      if (!(matches = vshReadlineCompletion(arg, 0, 0)))
>          goto cleanup;
>  
> -    for (iter = matches; *iter; iter++)
> +    for (n = 0, iter = matches; *iter; iter++, n++) {
> +        if (n == 0 && matches[1])
> +            continue;

This can be rewritten so that we don't need @n at all:

  if (iter == matches && matches[1])
      continue;

Or even better, just start iterating not from the first but second entry:

    for (iter = &matches[1]; *iter; iter++)
        printf("%s\n", *iter);

This is safe because:
a) @matches is guaranteed to be non-NULL at this point (due to check above),
b) @matches is NULL terminated array and as you and documentation [1]
say, the first entry is substitution for text (which we want to skip).

So even if the array is nothing but the substitution and NULL,
matches[1] will be NULL:

  matches = { "subs", NULL };

Also, I'm adding a small comment because it is not obvious why we are
skipping the first entry.

ACK then.

Michal

1: http://www.delorie.com/gnu/docs/readline/rlman_46.html