[libvirt] [PATCH] vsh: add a necessary assertion

[Marc Hartmayer]

On Thu, Jan 11, 2018 at 04:18 PM +0100, John Ferlan <jferlan at redhat.com> wrote:
> On 01/11/2018 10:02 AM, Marc Hartmayer wrote:
>> On Thu, Jan 04, 2018 at 08:20 PM +0100, John Ferlan <jferlan at redhat.com> wrote:
>>> On 12/18/2017 07:33 AM, Marc Hartmayer wrote:
>>>> This fixes the compilation error (compiled with the compiler option
>>>> '-03').
>>>>
>>>> In file included from ../../tools/vsh.c:28:0:
>>>> ../../tools/vsh.c: In function 'vshCommandOptStringQuiet':
>>>> ../../tools/vsh.c:838:30: error: potential null pointer dereference [-Werror=null-dereference]
>>>>      assert(!needData || valid->type != VSH_OT_BOOL);
>>>>
>>>> Signed-off-by: Marc Hartmayer <mhartmay at linux.vnet.ibm.com>
>>>> Reviewed-by: Boris Fiuczynski <fiuczy at linux.vnet.ibm.com>
>>>> ---
>>>>  tools/vsh.c | 6 ++++--
>>>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/tools/vsh.c b/tools/vsh.c
>>>> index e878119b988f..677eb9db3e41 100644
>>>> --- a/tools/vsh.c
>>>> +++ b/tools/vsh.c
>>>> @@ -816,8 +816,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 or the option was not found (which indicates a programming
>>>> + * error).  No error messages are issued if a value is returned.
>>>>   */
>>>>  static int
>>>>  vshCommandOpt(const vshCmd *cmd, const char *name, vshCmdOpt **opt,
>>>> @@ -835,6 +835,8 @@ vshCommandOpt(const vshCmd *cmd, const char *name, vshCmdOpt **opt,
>>>>              break;
>>>>          valid++;
>>>>      }
>>>> +    assert(valid);
>>>> +
>>>
>>> Could we combine the subsequent one to have :
>>>
>>> assert(valid && (!needData || valid->type != VSH_OT_BOOL));
>> 
>> Shall I resend a v2? Thanks.
>> 
>
> Let's see what Michal comes up with in his vsh series, see:
>
> https://www.redhat.com/archives/libvir-list/2018-January/msg00365.html
>
> I had seen his series, so I hesitated in pushing this until I (or
> someone else) had a chance to look at his series and determine if he
> addressed it at all.
>
> It's not like the assert issue is recent, so hopefully we can wait to
> see what he comes up with.

Sure :)

>
> John
>
>>>
>>> ?
>>>
>>> John
>>>
>>>>      assert(!needData || valid->type != VSH_OT_BOOL);
>>>>      if (valid->flags & VSH_OFLAG_REQ)
>>>>          ret = -1;
>>>>
>>>
>
-- 
Beste Grüße / Kind regards
   Marc Hartmayer

IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294