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

["Dumitrescu, Cristian" <cristian.dumitrescu@intel.com>]

> -----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