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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

>    was dereferencing "&& opt->type"
> 
> 3. Before dereferencing opt->completer, be sure opt isn't NULL.
> 
> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
> ---
>  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.

ACK

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]