Re: [dpdk-dev] [PATCH v3 5/9] mem: fix potential resource leak

[Bruce Richardson <bruce.richardson@intel.com>]

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 <anatoly.burakov@intel.com>
> > > ---
> > >   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