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

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.

>         if (STREQ(name, valid->name)) >             break; >         valid++; > diff --git a/tools/vsh.h b/tools/vsh.h > index 8f7df9ff8..112b1b57d 100644 > --- a/tools/vsh.h > +++ b/tools/vsh.h > @@ -188,7 +188,8 @@ struct _vshCmdDef { > struct _vshCmd { >     const vshCmdDef *def;       /* command definition */ >     vshCmdOpt *opts;            /* list of command arguments */ > -    vshCmd *next;      /* next command */ > +    vshCmd *next;               /* next command */ > +    bool skipChecks;            /* skip validity checks when > retrieving opts */ > }; > > /* > --  > 2.13.6 > > -- > libvir-list mailing list > libvir-list(a)redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list