On 05/11/2018 10:01 AM, Lin Ma wrote:
>
>
> 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?
Ah good catch. You're right, the documentation is not valid then. Sure,
if you can write the patch I'm happy to review it.
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
