Re: [dpdk-dev] [PATCH 1/1] lib/librte_eal: fix resource leak

[Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>]

On 20/04/2016 10:15, David Marchand wrote:
> On Tue, Apr 19, 2016 at 6:27 PM, Marcin Kerlin <marcinx.kerlin@intel.com> wrote:
>> Fix issue reported by Coverity.
>>
>> Coverity ID 13295, 13296, 13303:
>> Resource leak: The system resource will not be reclaimed
>> and reused, reducing the future availability of the resource.
>> In rte_eal_hugepage_attach: Leak of memory or pointers to system
>> resources.
>>
>> Fixes: af75078fece3 ("first public release")
>>
>> Signed-off-by: Marcin Kerlin <marcinx.kerlin@intel.com>
>> ---
>>   lib/librte_eal/linuxapp/eal/eal_memory.c | 12 +++++++++++-
>>   1 file changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/librte_eal/linuxapp/eal/eal_memory.c b/lib/librte_eal/linuxapp/eal/eal_memory.c
>> index 5b9132c..6320aa0 100644
>> --- a/lib/librte_eal/linuxapp/eal/eal_memory.c
>> +++ b/lib/librte_eal/linuxapp/eal/eal_memory.c
>> @@ -1475,13 +1475,17 @@ rte_eal_hugepage_attach(void)
>>                                          "and retry running both primary "
>>                                          "and secondary processes\n");
>>                          }
>> +
>> +                       if (base_addr != MAP_FAILED)
>> +                               munmap((void *)(uintptr_t)base_addr, mcfg->memseg[s].len);
>> +
> What is the point of this casting ?
> Idem for the rest of the patch.

I don't see the point either.
Marcin?

>
> I can't see cleanup for previously mapped segments when mapping one fails.
> Do we want this cleanup as well ?

Good point.

We are not really leaking resources because we panic if we fail to 
initialize eal memory.

Now, if we are going to do the cleanup, I think that as David points out 
we should be
cleaning up all previous mappings too.

Sergio
> CC Sergio.
>
>