Re: [dpdk-dev] [PATCH] eal: prevent dereferencing NULL pointer in rte_eal_devargs_add()

[Pawel Wodkowski <pawelx.wodkowski@intel.com>]

On 2015-03-02 15:40, Wiles, Keith wrote:
>
>
> On 3/2/15, 6:23 AM, "David Marchand" <david.marchand@6wind.com> wrote:
>
>> Hello Pawel,
>>
>> On Mon, Mar 2, 2015 at 12:09 PM, Pawel Wodkowski
>> <pawelx.wodkowski@intel.com
>>> wrote:
>>
>>> On failure devargs->args should not be accesed if devargs is NULL.
>>>
>>
>> accessed.
>>
>>
>>>
>>> Signed-off-by: Pawel Wodkowski <pawelx.wodkowski@intel.com>
>>> ---
>>>   lib/librte_eal/common/eal_common_devargs.c | 7 ++++---
>>>   1 file changed, 4 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/lib/librte_eal/common/eal_common_devargs.c
>>> b/lib/librte_eal/common/eal_common_devargs.c
>>> index 9b110f7..615945e 100644
>>> --- a/lib/librte_eal/common/eal_common_devargs.c
>>> +++ b/lib/librte_eal/common/eal_common_devargs.c
>>> @@ -124,12 +124,13 @@ rte_eal_devargs_add(enum rte_devtype devtype,
>>> const
>>> char *devargs_str)
>>>          return 0;
>>>
>>>   fail:
>>> -       if (devargs->args)
>>> -               free(devargs->args);
>>>          if (buf)
>>>                  free(buf);
>>> -       if (devargs)
>>> +       if (devargs) {
>>> +               free(devargs->args);
>
> Do you not still need to check for args being NULL before calling free?

No, there is no need for that. The same for buf. This NOP check is 
common practice in DPDK. I woul be good to clean this in whole library 
in separate patch set.

I recommend to read free() doc before doing another 'if (foo != NULL) 
free(foo)'

http://pubs.opengroup.org/onlinepubs/009695399/functions/free.html

-- 
Pawel