Re: [libvirt] [PATCH 08/18] vshCommandOpt: Allow caller avoiding assert()

On 01/11/2018 05:50 AM, Martin Kletzander wrote: > On Tue, Jan 02, 2018 at 06:12:01PM +0100, Michal Privoznik wrote: >> In the future, completer callbacks will receive partially parsed >> command (and thus possibly incomplete). However, we still want >> them to use command options fetching APIs we already have (e.g. >> vshCommandOpt*()) and at the same time don't report any errors >> (nor call any asserts). >> >> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; >> --- >> tools/vsh.c | 7 ++++--- >> tools/vsh.h | 3 ++- >> 2 files changed, 6 insertions(+), 4 deletions(-) >> >> diff --git a/tools/vsh.c b/tools/vsh.c >> index ebc8d9cb1..d27acb95b 100644 >> --- a/tools/vsh.c >> +++ b/tools/vsh.c >> @@ -815,8 +815,8 @@ vshCommandFree(vshCmd *cmd) >>  * to the option if found, 0 with *OPT set to NULL if the name is >>  * valid and the option is not required, -1 with *OPT set to NULL if >>  * the option is required but not present, and assert if NAME is not >> - * valid (which indicates a programming error).  No error messages are >> - * issued if a value is returned. >> + * valid (which indicates a programming error) unless cmd->skipChecks >> + * is set. No error messages are issued if a value is returned. >>  */ >> static int >> vshCommandOpt(const vshCmd *cmd, const char *name, vshCmdOpt **opt, >> @@ -829,7 +829,8 @@ vshCommandOpt(const vshCmd *cmd, const char *name, >> vshCmdOpt **opt, >>     /* See if option is valid and/or required.  */ >>     *opt = NULL; >>     while (valid) { >> -        assert(valid->name); >> +        if (!cmd->skipChecks) >> +            assert(valid->name); > > This can segfault when cmd->skipChecks == False && valid->name == NULL, > which is what the assert() guarded before. > > So either STREQ_NULLABLE or another if. > Hmmm... Also see: https://www.redhat.com/archives/libvir-list/2017-December/msg00605.html it's related somewhat...