1. Patchew
  2. Libvirt
  3. View series

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

Marc Hartmayer posted 1 patch 6 years, 4 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20171218123337.3855-1-mhartmay@linux.vnet.ibm.com
tools/vsh.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
[libvirt] [PATCH] vsh: add a necessary assertion
Posted by Marc Hartmayer 6 years, 4 months ago 
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@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@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);
+
     assert(!needData || valid->type != VSH_OT_BOOL);
     if (valid->flags & VSH_OFLAG_REQ)
         ret = -1;
-- 
2.13.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] vsh: add a necessary assertion
Posted by John Ferlan 6 years, 3 months ago 

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@linux.vnet.ibm.com>
> Reviewed-by: Boris Fiuczynski <fiuczy@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));

?

John

>      assert(!needData || valid->type != VSH_OT_BOOL);
>      if (valid->flags & VSH_OFLAG_REQ)
>          ret = -1;
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] vsh: add a necessary assertion
Posted by Marc Hartmayer 6 years, 3 months ago 
On Thu, Jan 04, 2018 at 08:20 PM +0100, John Ferlan <jferlan@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@linux.vnet.ibm.com>
>> Reviewed-by: Boris Fiuczynski <fiuczy@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));

I think so. Might make error tracking a little bit more difficult, but
that should be okay.

> ?
>
> 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


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] vsh: add a necessary assertion
Posted by Marc Hartmayer 6 years, 3 months ago 
On Thu, Jan 04, 2018 at 08:20 PM +0100, John Ferlan <jferlan@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@linux.vnet.ibm.com>
>> Reviewed-by: Boris Fiuczynski <fiuczy@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.

>
> ?
>
> 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


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] vsh: add a necessary assertion
Posted by John Ferlan 6 years, 3 months ago 

On 01/11/2018 10:02 AM, Marc Hartmayer wrote:
> On Thu, Jan 04, 2018 at 08:20 PM +0100, John Ferlan <jferlan@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@linux.vnet.ibm.com>
>>> Reviewed-by: Boris Fiuczynski <fiuczy@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.

John

>>
>> ?
>>
>> John
>>
>>>      assert(!needData || valid->type != VSH_OT_BOOL);
>>>      if (valid->flags & VSH_OFLAG_REQ)
>>>          ret = -1;
>>>
>>

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] vsh: add a necessary assertion
Posted by Marc Hartmayer 6 years, 3 months ago 
On Thu, Jan 11, 2018 at 04:18 PM +0100, John Ferlan <jferlan@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@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@linux.vnet.ibm.com>
>>>> Reviewed-by: Boris Fiuczynski <fiuczy@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


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.