Hi Adrien, > -----Original Message----- > From: Adrien Mazarguil [mailto:adrien.mazarg...@6wind.com] > Sent: Monday, January 30, 2017 9:33 AM > To: De Lara Guarch, Pablo > Cc: Wu, Jingjing; dev@dpdk.org > Subject: Re: [PATCH v2] app/testpmd: fix memory leak > > Hi Pablo, > > On Fri, Jan 27, 2017 at 02:54:58PM +0000, Pablo de Lara wrote: > > Free memory when port flow entry creation fails. > > > > Coverity issue: 139600 > > Fixes: 938a184a1870 ("app/testpmd: implement basic support for flow > API") > > > > Signed-off-by: Pablo de Lara <pablo.de.lara.gua...@intel.com> > > --- > > Changes in v2: > > > > - Removed unnecessary conditional > > > > app/test-pmd/config.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c > > index 5834498..467932f 100644 > > --- a/app/test-pmd/config.c > > +++ b/app/test-pmd/config.c > > @@ -954,6 +954,7 @@ port_flow_new(const struct rte_flow_attr *attr, > > goto store; > > } > > notsup: > > + free(pf); > > rte_errno = err; > > return NULL; > > } > > -- > > 2.7.4 > > > > I think this is a false positive, which is why I did not address it during > the last round of Coverity issues. > > As a two-pass function, errors are checked during the first pass, when pf is > not allocated yet. During the second pass, intermediate functions are not > supposed to return a different result and "notsup" cannot occur. > > I think assert()/rte_assert() would make more sense and should fool > Coverity > into understanding the expected behavior. > > The commit log should reflect that we are addressing a false positive since > there is no problem with the code logic (of course unless I missed anything > obvious).
Then, I would just ignore the coverity issue, changing the status to intentional. Thanks, Pablo > > Thanks. > > -- > Adrien Mazarguil > 6WIND
