On 3/2/15, 8:55 AM, "Wodkowski, PawelX" <pawelx.wodkowski at intel.com> wrote:
>On 2015-03-02 15:40, Wiles, Keith wrote:
>>
>>
>> On 3/2/15, 6:23 AM, "David Marchand" <david.marchand at 6wind.com> wrote:
>>
>>> Hello Pawel,
>>>
>>> On Mon, Mar 2, 2015 at 12:09 PM, Pawel Wodkowski
>>> <pawelx.wodkowski at intel.com
>>>> wrote:
>>>
>>>> On failure devargs->args should not be accesed if devargs is NULL.
>>>>
>>>
>>> accessed.
>>>
>>>
>>>>
>>>> Signed-off-by: Pawel Wodkowski <pawelx.wodkowski at 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
OK, did not realize this was changed. Do we know if all of the OSes DPDK
is built supports this free style?
I know that VxWorks did not support this free() method and I did port DPDK
to that OS, but it is not a supported platform for DPDK.
If some OS does not support passing NULL (and is supported by DPDK) to
free, then we need to abstract the free into a macro to allow those
systems to work correctly. I would expect using a macro for free would
also help if all frees were reworked to not test for NULL.
++Keith
>
>--
>Pawel
