From: Yunjian Wang <wangyunjian@huawei.com> Coverity issue: 357719 Fixes: da138cd47e06 ("net/octeontx2: handle port reconfigure") Cc: stable@dpdk.org Signed-off-by: Yunjian Wang <wangyunjian@huawei.com> --- drivers/net/octeontx2/otx2_ethdev.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/net/octeontx2/otx2_ethdev.c b/drivers/net/octeontx2/otx2_ethdev.c index 33b72bd4d..3f9399cc8 100644 --- a/drivers/net/octeontx2/otx2_ethdev.c +++ b/drivers/net/octeontx2/otx2_ethdev.c @@ -1355,8 +1355,6 @@ nix_store_queue_cfg_and_then_release(struct rte_eth_dev *eth_dev) fail: if (tx_qconf) free(tx_qconf); - if (rx_qconf) - free(rx_qconf); return -ENOMEM; } -- 2.23.0
On Wed, Aug 26, 2020 at 4:48 PM wangyunjian <wangyunjian@huawei.com> wrote: > > From: Yunjian Wang <wangyunjian@huawei.com> > > Coverity issue: 357719 > Fixes: da138cd47e06 ("net/octeontx2: handle port reconfigure") > Cc: stable@dpdk.org > > Signed-off-by: Yunjian Wang <wangyunjian@huawei.com> > --- > drivers/net/octeontx2/otx2_ethdev.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/net/octeontx2/otx2_ethdev.c b/drivers/net/octeontx2/otx2_ethdev.c > index 33b72bd4d..3f9399cc8 100644 > --- a/drivers/net/octeontx2/otx2_ethdev.c > +++ b/drivers/net/octeontx2/otx2_ethdev.c > @@ -1355,8 +1355,6 @@ nix_store_queue_cfg_and_then_release(struct rte_eth_dev *eth_dev) > fail: See below > if (tx_qconf) > free(tx_qconf); > - if (rx_qconf) > - free(rx_qconf); I think, it is clean and maintainable code have free() if rx_qconf as if we add some another exit error case in the future, we simply forget to add this check and it will fail. So I prefer to keep as-is for the sake of maintainability as there is no harm. > > return -ENOMEM; > } > -- > 2.23.0 > >
On Wed, 26 Aug 2020 19:18:09 +0800
wangyunjian <wangyunjian@huawei.com> wrote:
> if (tx_qconf)
> free(tx_qconf);
> - if (rx_qconf)
> - free(rx_qconf);
free accepts a NULL pointer so current practice is to avoid
useless if() tests before free.
> -----Original Message----- > From: Jerin Jacob [mailto:jerinjacobk@gmail.com] > Sent: Wednesday, September 30, 2020 2:15 AM > To: wangyunjian <wangyunjian@huawei.com> > Cc: dpdk-dev <dev@dpdk.org>; Jerin Jacob <jerinj@marvell.com>; Nithin > Dabilpuram <ndabilpuram@marvell.com>; Kiran Kumar K > <kirankumark@marvell.com>; Lilijun (Jerry) <jerry.lilijun@huawei.com>; > xudingke <xudingke@huawei.com>; dpdk stable <stable@dpdk.org> > Subject: Re: [dpdk-dev] [PATCH] net/octeontx2: remove logically dead code > > On Wed, Aug 26, 2020 at 4:48 PM wangyunjian <wangyunjian@huawei.com> > wrote: > > > > From: Yunjian Wang <wangyunjian@huawei.com> > > > > Coverity issue: 357719 > > Fixes: da138cd47e06 ("net/octeontx2: handle port reconfigure") > > Cc: stable@dpdk.org > > > > Signed-off-by: Yunjian Wang <wangyunjian@huawei.com> > > --- > > drivers/net/octeontx2/otx2_ethdev.c | 2 -- > > 1 file changed, 2 deletions(-) > > > > diff --git a/drivers/net/octeontx2/otx2_ethdev.c > > b/drivers/net/octeontx2/otx2_ethdev.c > > index 33b72bd4d..3f9399cc8 100644 > > --- a/drivers/net/octeontx2/otx2_ethdev.c > > +++ b/drivers/net/octeontx2/otx2_ethdev.c > > @@ -1355,8 +1355,6 @@ nix_store_queue_cfg_and_then_release(struct > > rte_eth_dev *eth_dev) > > fail: > > See below > > > if (tx_qconf) > > free(tx_qconf); > > - if (rx_qconf) > > - free(rx_qconf); > > I think, it is clean and maintainable code have free() if rx_qconf as if we add > some another exit error case in the future, we simply forget to add this check > and it will fail. So I prefer to keep as-is for the sake of maintainability as there is > no harm. Hi, Jerin Thanks for your explanation. According to Stephen's suggestion, is it need to remove unnecessary NULL check? Thanks, Yunjian > > > > > > return -ENOMEM; > > } > > -- > > 2.23.0 > > > >
On Wed, Sep 30, 2020 at 4:04 PM wangyunjian <wangyunjian@huawei.com> wrote: > > > -----Original Message----- > > From: Jerin Jacob [mailto:jerinjacobk@gmail.com] > > Sent: Wednesday, September 30, 2020 2:15 AM > > To: wangyunjian <wangyunjian@huawei.com> > > Cc: dpdk-dev <dev@dpdk.org>; Jerin Jacob <jerinj@marvell.com>; Nithin > > Dabilpuram <ndabilpuram@marvell.com>; Kiran Kumar K > > <kirankumark@marvell.com>; Lilijun (Jerry) <jerry.lilijun@huawei.com>; > > xudingke <xudingke@huawei.com>; dpdk stable <stable@dpdk.org> > > Subject: Re: [dpdk-dev] [PATCH] net/octeontx2: remove logically dead code > > > > On Wed, Aug 26, 2020 at 4:48 PM wangyunjian <wangyunjian@huawei.com> > > wrote: > > > > > > From: Yunjian Wang <wangyunjian@huawei.com> > > > > > > Coverity issue: 357719 > > > Fixes: da138cd47e06 ("net/octeontx2: handle port reconfigure") > > > Cc: stable@dpdk.org > > > > > > Signed-off-by: Yunjian Wang <wangyunjian@huawei.com> > > > --- > > > drivers/net/octeontx2/otx2_ethdev.c | 2 -- > > > 1 file changed, 2 deletions(-) > > > > > > diff --git a/drivers/net/octeontx2/otx2_ethdev.c > > > b/drivers/net/octeontx2/otx2_ethdev.c > > > index 33b72bd4d..3f9399cc8 100644 > > > --- a/drivers/net/octeontx2/otx2_ethdev.c > > > +++ b/drivers/net/octeontx2/otx2_ethdev.c > > > @@ -1355,8 +1355,6 @@ nix_store_queue_cfg_and_then_release(struct > > > rte_eth_dev *eth_dev) > > > fail: > > > > See below > > > > > if (tx_qconf) > > > free(tx_qconf); > > > - if (rx_qconf) > > > - free(rx_qconf); > > > > I think, it is clean and maintainable code have free() if rx_qconf as if we add > > some another exit error case in the future, we simply forget to add this check > > and it will fail. So I prefer to keep as-is for the sake of maintainability as there is > > no harm. > > Hi, Jerin > > Thanks for your explanation. > According to Stephen's suggestion, is it need to remove unnecessary NULL check? Yes. Makes sense to remove if check. > > Thanks, > Yunjian > > > > > > > > > > > return -ENOMEM; > > > } > > > -- > > > 2.23.0 > > > > > >
From: Yunjian Wang <wangyunjian@huawei.com> The glibc free allows free(NULL) as null operation, so remove this useless null checks. Coverity issue: 357719 Fixes: da138cd47e06 ("net/octeontx2: handle port reconfigure") Cc: stable@dpdk.org Signed-off-by: Yunjian Wang <wangyunjian@huawei.com> --- v2: fix code styles suggested by Stephen Hemminger --- drivers/net/octeontx2/otx2_ethdev.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/net/octeontx2/otx2_ethdev.c b/drivers/net/octeontx2/otx2_ethdev.c index 03d81faef..b69b92bf5 100644 --- a/drivers/net/octeontx2/otx2_ethdev.c +++ b/drivers/net/octeontx2/otx2_ethdev.c @@ -1383,10 +1383,8 @@ nix_store_queue_cfg_and_then_release(struct rte_eth_dev *eth_dev) return 0; fail: - if (tx_qconf) - free(tx_qconf); - if (rx_qconf) - free(rx_qconf); + free(tx_qconf); + free(rx_qconf); return -ENOMEM; } -- 2.23.0
On Fri, Oct 9, 2020 at 6:09 PM wangyunjian <wangyunjian@huawei.com> wrote: > > From: Yunjian Wang <wangyunjian@huawei.com> > > The glibc free allows free(NULL) as null operation, > so remove this useless null checks. > > Coverity issue: 357719 > Fixes: da138cd47e06 ("net/octeontx2: handle port reconfigure") > Cc: stable@dpdk.org > > Signed-off-by: Yunjian Wang <wangyunjian@huawei.com> Acked-by: Jerin Jacob <jerinj@marvell.com> Applied to dpdk-next-net-mrvl/for-main. Thanks > --- > v2: > fix code styles suggested by Stephen Hemminger > --- > drivers/net/octeontx2/otx2_ethdev.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/octeontx2/otx2_ethdev.c b/drivers/net/octeontx2/otx2_ethdev.c > index 03d81faef..b69b92bf5 100644 > --- a/drivers/net/octeontx2/otx2_ethdev.c > +++ b/drivers/net/octeontx2/otx2_ethdev.c > @@ -1383,10 +1383,8 @@ nix_store_queue_cfg_and_then_release(struct rte_eth_dev *eth_dev) > return 0; > > fail: > - if (tx_qconf) > - free(tx_qconf); > - if (rx_qconf) > - free(rx_qconf); > + free(tx_qconf); > + free(rx_qconf); > > return -ENOMEM; > } > -- > 2.23.0 >