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

[John Ferlan]

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 at redhat.com>
>> ---
>> 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...

John

>>         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 at redhat.com
>> https://www.redhat.com/mailman/listinfo/libvir-list
> 
> 
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>