[PATCH v3 00/10] devtools, lib, test, net, common: remove unused rte_bitmap_free()

[PATCH v3 00/10] devtools, lib, test, net, common: remove unused rte_bitmap_free()

[Ariel Otilibili]

Hello,

The first version  was about the clearing of Coverity IDs 357712 & 357737; now this series is about the removal of rte_bitmap_free().

As of commit 07604f2644 ("maintainers: update for next-net tree"):

* rte_bitmap_free() returns an integer, and does nothing else
* all functions that call rte_bitmap_free() do not use this return value.

Details are in a9486c43b3 ("devtools/cocci,lib/eal: remove unused rte_bitmap_free()").

Looking forward your feedback,

---
v3:
* addressed feedback from Stephen Hemminger
* removed rte_bitmap_free() wherever it was used

v2:
* fix style issues.

Ariel Otilibili (10):
  devtools/cocci,lib/eal: remove unused rte_bitmap_free()
  app/test: remove unsued rte_bitmap_free()
  net/sfc: remove unused rte_bitmap_free()
  crypto/ionic: remove unused rte_bitmap_free()
  net/cxgbe: remove unused rte_bitmap_free()
  net/mlx4: remove unused rte_bitmap_free()
  common/mlx5: remove unused rte_bitmap_free()
  net/bonding: remove unused rte_bitmap_free()
  net/netvsc: remove unused rte_bitmap_free()
  common/cnxk: remove unused rte_bitmap_free()

 app/test/test_bitmap.c                   |  2 --
 devtools/cocci/nullfree.cocci            |  3 ---
 drivers/common/cnxk/roc_platform.h       |  1 -
 drivers/common/mlx5/mlx5_common_mr.c     |  1 -
 drivers/crypto/ionic/ionic_crypto_main.c |  1 -
 drivers/net/bonding/rte_eth_bond_pmd.c   |  1 -
 drivers/net/cxgbe/cxgbe_main.c           |  1 -
 drivers/net/mlx4/mlx4_mr.c               |  1 -
 drivers/net/netvsc/hn_rxtx.c             |  1 -
 drivers/net/sfc/sfc_sw_stats.c           |  1 -
 lib/eal/include/rte_bitmap.h             | 19 -------------------
 11 files changed, 32 deletions(-)

-- 
2.47.1

[PATCH v3 01/10] devtools/cocci, lib/eal: remove unused rte_bitmap_free()

[Ariel Otilibili]

Coverity reports return values of calls to rte_bitmap_free() are not used.

As of commit 07604f2644 ("maintainers: update for next-net tree"):

```
$ awk 'NR>281 && NR<300 {print NR ":" $0}' lib/eal/include/rte_bitmap.h

282:/**
283: * Bitmap free
284: *
285: * @param bmp
286: *   Handle to bitmap instance
287: * @return
288: *   0 upon success, error code otherwise
289: */
290:static inline int
291:rte_bitmap_free(struct rte_bitmap *bmp)
292:{
293:    /* Check input arguments */
294:    if (bmp == NULL) {
295:            return -1;
296:    }
297:
298:    return 0;
299:}
```

rte_bitmap_free() checks the pointer to its input rte_bitmap is not null,
and does nothing else.

The functions wherein rte_bitmap_free() is called do not use its return value.

```
$ git grep -Pn 'rte_bitmap_free\('

app/test/test_bitmap.c:211:     rte_bitmap_free(bmp);
app/test/test_bitmap.c:257:     rte_bitmap_free(bmp);
devtools/cocci/nullfree.cocci:19:- if (E != NULL) rte_bitmap_free(E);
devtools/cocci/nullfree.cocci:20:+ rte_bitmap_free(E);
drivers/common/cnxk/roc_platform.h:127:#define plt_bitmap_free                  rte_bitmap_free
drivers/common/mlx5/mlx5_common_mr.c:497:       rte_bitmap_free(mr->ms_bmp);
drivers/crypto/ionic/ionic_crypto_main.c:817:           rte_bitmap_free(dev->sess_bm);
drivers/net/bonding/rte_eth_bond_pmd.c:2246:    rte_bitmap_free(internals->vlan_filter_bmp);
drivers/net/cxgbe/cxgbe_main.c:468:             rte_bitmap_free(t->ftid_bmap);
drivers/net/mlx4/mlx4_mr.c:474: rte_bitmap_free(mr->ms_bmp);
drivers/net/netvsc/hn_rxtx.c:186:       rte_bitmap_free(hv->chim_bmap);
drivers/net/sfc/sfc_sw_stats.c:818:     rte_bitmap_free(sa->sw_stats.queues_bitmap);
lib/eal/include/rte_bitmap.h:291:rte_bitmap_free(struct rte_bitmap *bmp)
```

The clearing of these Coverity warnings have been discussed more than once this
year, the question of its existence has also been asked.

Subsequent commits will remove rte_bitmap_free where it was used.

Coverity issue: 357712, 357737
Link: https://lore.kernel.org/all/6A08397F-8D6E-4631-B63E-4CAE319F1463@amd.com/T/#m35464e4e6dccfa904ab7e5f0a0b246e63e0ed74e
Link: https://inbox.dpdk.org/dev/20241213085928.0f2f2de1@hermes.local/
Signed-off-by: Ariel Otilibili <otilibil@eurecom.fr>
---
Cc: stable@dpdk.org
Cc: Stephen Hemminger stephen@networkplumber.org
Cc: Andrew Boyer <andrew.boyer@amd.com>
---
 devtools/cocci/nullfree.cocci |  3 ---
 lib/eal/include/rte_bitmap.h  | 19 -------------------
 2 files changed, 22 deletions(-)

diff --git a/devtools/cocci/nullfree.cocci b/devtools/cocci/nullfree.cocci
index c0526a2a3f..8f0c4a4144 100644
--- a/devtools/cocci/nullfree.cocci
+++ b/devtools/cocci/nullfree.cocci
@@ -16,9 +16,6 @@ expression E;
 - if (E != NULL) rte_acl_free(E);
 + rte_acl_free(E);
 |
-- if (E != NULL) rte_bitmap_free(E);
-+ rte_bitmap_free(E);
-|
 - if (E != NULL) rte_comp_op_free(E);
 + rte_comp_op_free(E);
 |
diff --git a/lib/eal/include/rte_bitmap.h b/lib/eal/include/rte_bitmap.h
index abb102f1d3..63d54f5761 100644
--- a/lib/eal/include/rte_bitmap.h
+++ b/lib/eal/include/rte_bitmap.h
@@ -279,25 +279,6 @@ rte_bitmap_init_with_all_set(uint32_t n_bits, uint8_t *mem, uint32_t mem_size)
 	return bmp;
 }
 
-/**
- * Bitmap free
- *
- * @param bmp
- *   Handle to bitmap instance
- * @return
- *   0 upon success, error code otherwise
- */
-static inline int
-rte_bitmap_free(struct rte_bitmap *bmp)
-{
-	/* Check input arguments */
-	if (bmp == NULL) {
-		return -1;
-	}
-
-	return 0;
-}
-
 /**
  * Bitmap reset
  *
-- 
2.47.1

[PATCH v3 02/10] app/test: remove unsued rte_bitmap_free()

[Ariel Otilibili]

Depends on 17dd8149d6 ("devtools/cocci,lib/eal: remove unused
rte_bitmap_free()").

Coverity issue: 357712, 357737
Fixes: 24d376bfee ("test/bitmap: fix memory leak")
Signed-off-by: Ariel Otilibili <otilibil@eurecom.fr>
--
Cc: stable@dpdk.org
Cc: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
Cc: Stephen Hemminger <stephen@networkplumber.org>
---
 app/test/test_bitmap.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/app/test/test_bitmap.c b/app/test/test_bitmap.c
index bab11812c7..a21210a215 100644
--- a/app/test/test_bitmap.c
+++ b/app/test/test_bitmap.c
@@ -208,7 +208,6 @@ test_bitmap_all_clear(void)
 	if (test_bitmap_scan_operations(bmp) < 0)
 		return TEST_FAILED;
 
-	rte_bitmap_free(bmp);
 	rte_free(mem);
 
 	return TEST_SUCCESS;
@@ -254,7 +253,6 @@ test_bitmap_all_set(void)
 		return TEST_FAILED;
 	}
 
-	rte_bitmap_free(bmp);
 	rte_free(mem);
 
 	return TEST_SUCCESS;
-- 
2.47.1

[PATCH v3 03/10] net/sfc: remove unused rte_bitmap_free()

[Ariel Otilibili]

Depends on 17dd8149d6 ("devtools/cocci,lib/eal: remove unused
rte_bitmap_free()").

Fixes: e00c3a0c1b ("net/sfc: rename SW stats structures")
Signed-off-by: Ariel Otilibili <otilibil@eurecom.fr>
---
Cc: stable@dpdk.org
Cc: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
---
 drivers/net/sfc/sfc_sw_stats.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/sfc/sfc_sw_stats.c b/drivers/net/sfc/sfc_sw_stats.c
index 3ae5023b6f..90e95aed22 100644
--- a/drivers/net/sfc/sfc_sw_stats.c
+++ b/drivers/net/sfc/sfc_sw_stats.c
@@ -815,7 +815,6 @@ sfc_sw_xstats_configure(struct sfc_adapter *sa)
 static void
 sfc_sw_xstats_free_queues_bitmap(struct sfc_adapter *sa)
 {
-	rte_bitmap_free(sa->sw_stats.queues_bitmap);
 	rte_free(sa->sw_stats.queues_bitmap_mem);
 }
 
-- 
2.47.1

[PATCH v3 04/10] crypto/ionic: remove unused rte_bitmap_free()

[Ariel Otilibili]

Depends on 17dd8149d6 ("devtools/cocci,lib/eal: remove unused
rte_bitmap_free()").

Fixes: 6bc7f2cf66 ("crypto/ionic: support sessions")
Signed-off-by: Ariel Otilibili <otilibil@eurecom.fr>
---
Cc: stable@dpdk.org
Cc: Andrew Boyer <andrew.boyer@amd.com>
---
 drivers/crypto/ionic/ionic_crypto_main.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/crypto/ionic/ionic_crypto_main.c b/drivers/crypto/ionic/ionic_crypto_main.c
index f670d6e658..ef5b54f161 100644
--- a/drivers/crypto/ionic/ionic_crypto_main.c
+++ b/drivers/crypto/ionic/ionic_crypto_main.c
@@ -814,7 +814,6 @@ iocpt_free_objs(struct iocpt_dev *dev)
 	}
 
 	if (dev->sess_bm != NULL) {
-		rte_bitmap_free(dev->sess_bm);
 		rte_free(dev->sess_bm);
 		dev->sess_bm = NULL;
 	}
-- 
2.47.1

[PATCH v3 05/10] net/cxgbe: remove unused rte_bitmap_free()

[Ariel Otilibili]

Depends on 17dd8149d6 ("devtools/cocci,lib/eal: remove unused
rte_bitmap_free()").

Fixes: 06c047b680 ("remove unnecessary null checks")
Signed-off-by: Ariel Otilibili <otilibil@eurecom.fr>
---
Cc: stable@dpdk.org
Cc: Potnuri Bharat Teja <bharat@chelsio.com>
---
 drivers/net/cxgbe/cxgbe_main.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/cxgbe/cxgbe_main.c b/drivers/net/cxgbe/cxgbe_main.c
index 2ed21f2d66..91ecddd287 100644
--- a/drivers/net/cxgbe/cxgbe_main.c
+++ b/drivers/net/cxgbe/cxgbe_main.c
@@ -465,7 +465,6 @@ void cxgbe_insert_tid(struct tid_info *t, void *data, unsigned int tid,
 static void tid_free(struct tid_info *t)
 {
 	if (t->tid_tab) {
-		rte_bitmap_free(t->ftid_bmap);
 
 		if (t->ftid_bmap_array)
 			t4_os_free(t->ftid_bmap_array);
-- 
2.47.1

[PATCH v3 06/10] net/mlx4: remove unused rte_bitmap_free()

[Ariel Otilibili]

Depends on 17dd8149d6 ("devtools/cocci,lib/eal: remove unused
rte_bitmap_free()").

Fixes: 06c047b680 ("remove unnecessary null checks")
Signed-off-by: Ariel Otilibili <otilibil@eurecom.fr>
---
Cc: stable@dpdk.org
Cc: Matan Azrad <matan@nvidia.com>
Cc: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
---
 drivers/net/mlx4/mlx4_mr.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/mlx4/mlx4_mr.c b/drivers/net/mlx4/mlx4_mr.c
index 2251c624ff..67a2a66481 100644
--- a/drivers/net/mlx4/mlx4_mr.c
+++ b/drivers/net/mlx4/mlx4_mr.c
@@ -471,7 +471,6 @@ mr_free(struct mlx4_mr *mr)
 	DEBUG("freeing MR(%p):", (void *)mr);
 	if (mr->ibv_mr != NULL)
 		claim_zero(mlx4_glue->dereg_mr(mr->ibv_mr));
-	rte_bitmap_free(mr->ms_bmp);
 	rte_free(mr);
 }
 
-- 
2.47.1

[PATCH v3 07/10] common/mlx5: remove unused rte_bitmap_free()

[Ariel Otilibili]

Depends on 17dd8149d6 ("devtools/cocci,lib/eal: remove unused
rte_bitmap_free()").

Fixes: 06c047b680 ("remove unnecessary null checks")
Signed-off-by: Ariel Otilibili <otilibil@eurecom.fr>
---
Cc: stable@dpdk.org
Cc: Dariusz Sosnowski <dsosnowski@nvidia.com>
Cc: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
Cc: Bing Zhao <bingz@nvidia.com>
Cc: Ori Kam <orika@nvidia.com>
Cc: Suanming Mou <suanmingm@nvidia.com>
Cc: Matan Azrad <matan@nvidia.com>
---
 drivers/common/mlx5/mlx5_common_mr.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/common/mlx5/mlx5_common_mr.c b/drivers/common/mlx5/mlx5_common_mr.c
index 50922ad398..dee0b521dd 100644
--- a/drivers/common/mlx5/mlx5_common_mr.c
+++ b/drivers/common/mlx5/mlx5_common_mr.c
@@ -494,7 +494,6 @@ mlx5_mr_free(struct mlx5_mr *mr, mlx5_dereg_mr_t dereg_mr_cb)
 		return;
 	DRV_LOG(DEBUG, "freeing MR(%p):", (void *)mr);
 	dereg_mr_cb(&mr->pmd_mr);
-	rte_bitmap_free(mr->ms_bmp);
 	mlx5_free(mr);
 }
 
-- 
2.47.1

[PATCH v3 08/10] net/bonding: remove unused rte_bitmap_free()

[Ariel Otilibili]

Depends on 17dd8149d6 ("devtools/cocci,lib/eal: remove unused
rte_bitmap_free()").

Fixes: 171875d067 ("net/bonding: release port upon close")
Signed-off-by: Ariel Otilibili <otilibil@eurecom.fr>
---
Cc: stable@dpdk.org
Cc: Chas Williams <chas3@att.com>
Cc: "Min Hu (Connor)" <humin29@huawei.com>
---
 drivers/net/bonding/rte_eth_bond_pmd.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
index 91bf2c2345..5f9a739e55 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -2243,7 +2243,6 @@ bond_ethdev_close(struct rte_eth_dev *dev)
 
 	bond_ethdev_free_queues(dev);
 	rte_bitmap_reset(internals->vlan_filter_bmp);
-	rte_bitmap_free(internals->vlan_filter_bmp);
 	rte_free(internals->vlan_filter_bmpmem);
 
 	/* Try to release mempool used in mode6. If the bond
-- 
2.47.1

[PATCH v3 09/10] net/netvsc: remove unused rte_bitmap_free()

[Ariel Otilibili]

Depends on 17dd8149d6 ("devtools/cocci,lib/eal: remove unused
rte_bitmap_free()").

Fixes: cc02518132 ("net/netvsc: split send buffers from Tx descriptors")
Signed-off-by: Ariel Otilibili <otilibil@eurecom.fr>
---
Cc: stable@dpdk.org
Cc: Long Li <longli@microsoft.com>
Cc: Wei Hu <weh@microsoft.com>
---
 drivers/net/netvsc/hn_rxtx.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/netvsc/hn_rxtx.c b/drivers/net/netvsc/hn_rxtx.c
index 9d3948e03d..148c7c4a33 100644
--- a/drivers/net/netvsc/hn_rxtx.c
+++ b/drivers/net/netvsc/hn_rxtx.c
@@ -183,7 +183,6 @@ hn_chim_uninit(struct rte_eth_dev *dev)
 {
 	struct hn_data *hv = dev->data->dev_private;
 
-	rte_bitmap_free(hv->chim_bmap);
 	rte_free(hv->chim_bmem);
 	hv->chim_bmem = NULL;
 }
-- 
2.47.1

[PATCH v3 10/10] common/cnxk: remove unused rte_bitmap_free()

[Ariel Otilibili]

Depends on 17dd8149d6 ("devtools/cocci,lib/eal: remove unused
rte_bitmap_free()").

Fixes: fa8f86a14e ("common/cnxk: add build infrastructre and HW definition")
Signed-off-by: Ariel Otilibili <otilibil@eurecom.fr>
---
Cc: stable@dpdk.org
Cc: Nithin Dabilpuram <ndabilpuram@marvell.com>
Cc: Kiran Kumar K <kirankumark@marvell.com>
Cc: Sunil Kumar Kori <skori@marvell.com>
Cc: Satha Rao <skoteshwar@marvell.com>
Cc: Harman Kalra <hkalra@marvell.com>
---
 drivers/common/cnxk/roc_platform.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/common/cnxk/roc_platform.h b/drivers/common/cnxk/roc_platform.h
index df4f88f288..6c4a69377f 100644
--- a/drivers/common/cnxk/roc_platform.h
+++ b/drivers/common/cnxk/roc_platform.h
@@ -124,7 +124,6 @@
 #define plt_bitmap			rte_bitmap
 #define plt_bitmap_init			rte_bitmap_init
 #define plt_bitmap_reset		rte_bitmap_reset
-#define plt_bitmap_free			rte_bitmap_free
 #define plt_bitmap_clear		rte_bitmap_clear
 #define plt_bitmap_set			rte_bitmap_set
 #define plt_bitmap_get			rte_bitmap_get
-- 
2.47.1

Re: [PATCH v3 01/10] devtools/cocci, lib/eal: remove unused rte_bitmap_free()

[Stephen Hemminger]

On Sun, 22 Dec 2024 02:19:53 +0100
Ariel Otilibili <otilibil@eurecom.fr> wrote:

> Coverity reports return values of calls to rte_bitmap_free() are not used.

Some existing user code may actually be using this so this would be an API change.
Therefore this patch can not be accepted for current releases.

It is worth addressing this in future but:
  - need to go through deprecation process
  - function to be void
  - it should be possible to implement a real free function
    just was never done.