On 20/04/2016 10:15, David Marchand wrote: > On Tue, Apr 19, 2016 at 6:27 PM, Marcin Kerlin <marcinx.kerlin at 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 at 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. > >