From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id F2EE03990 for ; Thu, 21 Apr 2016 13:19:20 +0200 (CEST) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga104.fm.intel.com with ESMTP; 21 Apr 2016 04:19:20 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,512,1455004800"; d="scan'208";a="963446287" Received: from smonroyx-mobl.ger.corp.intel.com (HELO [10.237.221.26]) ([10.237.221.26]) by fmsmga002.fm.intel.com with ESMTP; 21 Apr 2016 04:19:03 -0700 From: Sergio Gonzalez Monroy To: David Marchand Cc: "dev@dpdk.org" , Marcin Kerlin References: <1461083251-31140-1-git-send-email-marcinx.kerlin@intel.com> Message-ID: <5718B726.5040300@intel.com> Date: Thu, 21 Apr 2016 12:19:02 +0100 User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.7.2 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH 1/1] lib/librte_eal: fix resource leak X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 21 Apr 2016 11:19:21 -0000 On 20/04/2016 10:15, David Marchand wrote: > On Tue, Apr 19, 2016 at 6:27 PM, Marcin Kerlin 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 >> --- >> 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. > >