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@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