On 05/10/2018 05:17 PM, Michal Privoznik wrote:
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@xxxxxxxx>
---
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);
It seems it can't handle the case that no command or option share the
same prefix.
say 'event' command, when users type virsh ev<TAB><TAB>, there is no
other command
sharing 'ev' prefix, in this case, the matches[1] is NULL and we need to
print the
value in matches[0],I think we can't skip the first entry in this case.
So the code block 'if (iter == matches && matches[1]) continue;' looks
like a better
choice.If you think so,
I'd like to write a patch to do it, Do you agree?
meanwhile, I want to remove those comment due to we can't skip first
entry, Do you agree?
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
Thanks,
Lin
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
