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@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. > Yep - couple shots of tequila or whatever that Borovicka stuff is we had at KVM Forum was could help ;-) John -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list