[libvirt] [PATCH 5/7] vsh: Fix some issues in auto completion code

[John Ferlan]

On 10/10/2016 12:33 PM, Peter Krempa wrote:
> On Mon, Oct 10, 2016 at 11:42:16 -0400, John Ferlan wrote:
>> 1. Move the declaration of const vshCmdDef *help - it should be at the
>>    top of the "if" rather than in the middle.
>>
>> 2. Change a comparison from && to || - without doing so we could core
> 
> Same comment as in 3/7 on 'core' not being the appropriate term.
> 
>>    on commands like 'virsh list' which would allow completion of some
>>    non -- option based on whatever was found in the current working
>>    directory and then as soon as that was completed, the next <tab>
>>    would core since "opt" would be returned as NULL, but the check
> 
> aaand again.
> 

OK changed them to crash.

>>    was dereferencing "&& opt->type"
>>
>> 3. Before dereferencing opt->completer, be sure opt isn't NULL.
>>
>> Signed-off-by: John Ferlan <jferlan at redhat.com>
>> ---
>>  tools/vsh.c | 7 ++++---
>>  1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/vsh.c b/tools/vsh.c
>> index 420eb79..9558dad 100644
>> --- a/tools/vsh.c
>> +++ b/tools/vsh.c
>> @@ -1523,10 +1523,11 @@ vshCommandParse(vshControl *ctl, vshCommandParser *parser)
>>              /* if we encountered --help, replace parsed command with
>>               * 'help <cmdname>' */
>>              for (tmpopt = first; tmpopt; tmpopt = tmpopt->next) {
>> +                const vshCmdDef *help;
>>                  if (STRNEQ(tmpopt->def->name, "help"))
>>                      continue;
>>  
>> -                const vshCmdDef *help = vshCmddefSearch("help");
>> +                help = vshCmddefSearch("help");
>>                  vshCommandOptFree(first);
>>                  first = vshMalloc(ctl, sizeof(vshCmdOpt));
>>                  first->def = help->opts;
>> @@ -2788,7 +2789,7 @@ vshReadlineParse(const char *text, int state)
>>                   * Try to find the default option.
>>                   */
>>                  if (!(opt = vshCmddefGetData(cmd, &opts_need_arg, &opts_seen))
>> -                    && opt->type == VSH_OT_BOOL)
>> +                    || opt->type == VSH_OT_BOOL)
>>                      goto error;
>>                  opt_exists = true;
>>                  opts_need_arg = const_opts_need_arg;
>> @@ -2824,7 +2825,7 @@ vshReadlineParse(const char *text, int state)
>>          res = vshReadlineCommandGenerator(sanitized_text, state);
>>      } else if (opts_filled && !non_bool_opt_exists) {
>>          res = vshReadlineOptionsGenerator(sanitized_text, state, cmd);
>> -    } else if (non_bool_opt_exists && data_complete && opt->completer) {
>> +    } else if (non_bool_opt_exists && data_complete && opt && opt->completer) {
>>          if (!completed_list)
>>              completed_list = opt->completer(autoCompleteOpaque,
>>                                              opt->completer_flags);
> 
> This function makes my eyes bleed.
> 

Yep - couple shots of tequila or whatever that Borovicka stuff is we had
at KVM Forum was could help ;-)

John