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

John Ferlan jferlan at redhat.com
Thu Jan 11 13:09:25 UTC 2018



On 01/11/2018 07:55 AM, Martin Kletzander wrote:
> On Thu, Jan 11, 2018 at 06:02:58AM -0500, John Ferlan wrote:
>>
>>
>> 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...
>>
> 
> I don't see how, this is all wrapped in `while (valid)`

The other patch is "after" the loop.

Look at the entire context... although we know it's a software
engineering error to not have some sort of match, some compiler believes
we can exit the "while (valid)" loop with "valid == NULL", follwed by
the next assert which dereferences @valid without asserting if valid is
non-NULL.

I didn't say it was exactly related, just "related somewhat".

John




More information about the libvir-list mailing list