From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by dpdk.org (Postfix) with ESMTP id 1F6EF8E01 for ; Fri, 27 Apr 2018 17:52:16 +0200 (CEST) X-Amp-Result: UNSCANNABLE X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by orsmga106.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 27 Apr 2018 08:52:15 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.49,335,1520924400"; d="scan'208";a="41134686" Received: from bricha3-mobl.ger.corp.intel.com ([10.237.221.51]) by fmsmga002.fm.intel.com with SMTP; 27 Apr 2018 08:52:13 -0700 Received: by (sSMTP sendmail emulation); Fri, 27 Apr 2018 16:52:13 +0100 Date: Fri, 27 Apr 2018 16:52:12 +0100 From: Bruce Richardson To: "Burakov, Anatoly" Cc: dev@dpdk.org, thomas@monjalon.net Message-ID: <20180427155212.GB94056@bricha3-MOBL.ger.corp.intel.com> References: <36228cdd42eef261936b07c42a3c582f7e715da1.1524650130.git.anatoly.burakov@intel.com> <20180427152116.GE80648@bricha3-MOBL.ger.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Organization: Intel Research and Development Ireland Ltd. User-Agent: Mutt/1.9.4 (2018-02-28) Subject: Re: [dpdk-dev] [PATCH v3 5/9] mem: fix potential resource leak X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 27 Apr 2018 15:52:17 -0000 On Fri, Apr 27, 2018 at 04:49:03PM +0100, Burakov, Anatoly wrote: > On 27-Apr-18 4:21 PM, Bruce Richardson wrote: > > On Wed, Apr 25, 2018 at 10:56:43AM +0100, Anatoly Burakov wrote: > > > Normally, tailq entry should have a valid fd by the time we attempt > > > to map the segment. However, in case it doesn't, we're leaking fd, > > > so fix it. > > > > > > Coverity issue: 272570 > > > > > > Fixes: 2a04139f66b4 ("eal: add single file segments option") > > > Cc: anatoly.burakov@intel.com > > > > > > Signed-off-by: Anatoly Burakov > > > --- > > > lib/librte_eal/linuxapp/eal/eal_memalloc.c | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/lib/librte_eal/linuxapp/eal/eal_memalloc.c b/lib/librte_eal/linuxapp/eal/eal_memalloc.c > > > index fab5a98..b02e3a5 100644 > > > --- a/lib/librte_eal/linuxapp/eal/eal_memalloc.c > > > +++ b/lib/librte_eal/linuxapp/eal/eal_memalloc.c > > > @@ -524,6 +524,8 @@ alloc_seg(struct rte_memseg *ms, void *addr, int socket_id, > > > if (te != NULL && te->fd >= 0) { > > > close(te->fd); > > > te->fd = -1; > > > > Is "fd" still not being leaked here, since we won't hit the else case and > > then jump to the end of the function where it goes out of scope? > > Technically, the "else" case is never valid here. If we have a tailq entry - > we always have a valid fd. So perhaps it should be classified as a false > positive. > If there is a (non-harmful) way to fix it in the code, I'd definitely thing we should do so. It means that any other projects which use coverity scans, or other static analysis scans, on DPDK code won't have to re-disposition the issue. Also, if we ever start having separate scans of the different trees, we'll similarly see benefit of not having to mark false positives multiple times. /Bruce