[dpdk-dev,2/2] rte_sched: remove useless bitmap_free

Message ID 1440780599-14851-3-git-send-email-stephen@networkplumber.org (mailing list archive)
State Changes Requested, archived
Headers

Commit Message

Stephen Hemminger Aug. 28, 2015, 4:49 p.m. UTC
  Coverity reports that rte_bitmap_free() does nothing and caller does
not check return value. Just remove it.

Also since rte_free(NULL) is a nop, remove useless check here.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_sched/rte_bitmap.h | 19 -------------------
 lib/librte_sched/rte_sched.c  |  5 -----
 2 files changed, 24 deletions(-)
  

Comments

Cristian Dumitrescu Sept. 11, 2015, 5:28 p.m. UTC | #1
> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Friday, August 28, 2015 7:50 PM
> To: Dumitrescu, Cristian
> Cc: dev@dpdk.org; Stephen Hemminger
> Subject: [PATCH 2/2] rte_sched: remove useless bitmap_free
> 
> Coverity reports that rte_bitmap_free() does nothing and caller does
> not check return value. Just remove it.
> 
> Also since rte_free(NULL) is a nop, remove useless check here.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  lib/librte_sched/rte_bitmap.h | 19 -------------------
>  lib/librte_sched/rte_sched.c  |  5 -----
>  2 files changed, 24 deletions(-)
> 
> diff --git a/lib/librte_sched/rte_bitmap.h b/lib/librte_sched/rte_bitmap.h
> index 216a344..47eeeeb 100644
> --- a/lib/librte_sched/rte_bitmap.h
> +++ b/lib/librte_sched/rte_bitmap.h
> @@ -275,25 +275,6 @@ rte_bitmap_init(uint32_t n_bits, uint8_t *mem,
> uint32_t mem_size)
>  }
> 
>  /**
> - * 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
>   *
>   * @param bmp
> diff --git a/lib/librte_sched/rte_sched.c b/lib/librte_sched/rte_sched.c
> index 924c172..cbe3f3b 100644
> --- a/lib/librte_sched/rte_sched.c
> +++ b/lib/librte_sched/rte_sched.c
> @@ -716,11 +716,6 @@ rte_sched_port_config(struct
> rte_sched_port_params *params)
>  void
>  rte_sched_port_free(struct rte_sched_port *port)
>  {
> -	/* Check user parameters */
> -	if (port == NULL)
> -		return;
> -
> -	rte_bitmap_free(port->bmp);
>  	rte_free(port);
>  }
> 
> --
> 2.1.4

Hi Steve,

I agree these functions are not doing much at the moment, but I would like to keep them for the reasons below:

1. There might be people using them, and we do not want to break their code. Removing them is an ABI change.

2. Although they are just placeholders for now, we might need to add more functionality to them going forward, as the implementation evolves. I don't want to change the API now by removing them, and change the API later when we need to add them back. Generally, I think it is good practice to have free functions.

Regards,
Cristian
  
Stephen Hemminger Sept. 11, 2015, 7:18 p.m. UTC | #2
> Hi Steve,
> 
> I agree these functions are not doing much at the moment, but I would like to keep them for the reasons below:
> 
> 1. There might be people using them, and we do not want to break their code. Removing them is an ABI change.
> 
> 2. Although they are just placeholders for now, we might need to add more functionality to them going forward, as the implementation evolves. I don't want to change the API now by removing them, and change the API later when we need to add them back. Generally, I think it is good practice to have free functions.

The source code base is not a code repository for unused and dead code!
If you need to keep things, either get them out of previous history with git
or keep them out of tree.

Also have a number of patches to remove all #ifdef code that is dead
in QoS now.
  

Patch

diff --git a/lib/librte_sched/rte_bitmap.h b/lib/librte_sched/rte_bitmap.h
index 216a344..47eeeeb 100644
--- a/lib/librte_sched/rte_bitmap.h
+++ b/lib/librte_sched/rte_bitmap.h
@@ -275,25 +275,6 @@  rte_bitmap_init(uint32_t n_bits, uint8_t *mem, uint32_t mem_size)
 }
 
 /**
- * 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
  *
  * @param bmp
diff --git a/lib/librte_sched/rte_sched.c b/lib/librte_sched/rte_sched.c
index 924c172..cbe3f3b 100644
--- a/lib/librte_sched/rte_sched.c
+++ b/lib/librte_sched/rte_sched.c
@@ -716,11 +716,6 @@  rte_sched_port_config(struct rte_sched_port_params *params)
 void
 rte_sched_port_free(struct rte_sched_port *port)
 {
-	/* Check user parameters */
-	if (port == NULL)
-		return;
-
-	rte_bitmap_free(port->bmp);
 	rte_free(port);
 }