diff mbox series

[06/82] examples/pipeline/thread: remove unnecessary NULL checks

Message ID 20220124000518.319850-7-stephen@networkplumber.org (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers show
Series remove unnecessary null checks | expand

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Stephen Hemminger Jan. 24, 2022, 12:04 a.m. UTC
Remove redundant NULL pointer checks before free functions
found by nullfree.cocci

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 examples/pipeline/thread.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

Comments

Cristian Dumitrescu Jan. 24, 2022, 10:18 a.m. UTC | #1
> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Monday, January 24, 2022 12:04 AM
> To: dev@dpdk.org
> Cc: Stephen Hemminger <stephen@networkplumber.org>; Dumitrescu,
> Cristian <cristian.dumitrescu@intel.com>
> Subject: [PATCH 06/82] examples/pipeline/thread: remove unnecessary
> NULL checks
> 
> Remove redundant NULL pointer checks before free functions
> found by nullfree.cocci
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  examples/pipeline/thread.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/examples/pipeline/thread.c b/examples/pipeline/thread.c
> index bf11e6144b75..5fe7eae00eb6 100644
> --- a/examples/pipeline/thread.c
> +++ b/examples/pipeline/thread.c
> @@ -89,11 +89,9 @@ thread_free(void)
>  			continue;
> 
>  		/* MSGQs */
> -		if (t->msgq_req)
> -			rte_ring_free(t->msgq_req);
> +		rte_ring_free(t->msgq_req);
> 
> -		if (t->msgq_rsp)
> -			rte_ring_free(t->msgq_rsp);
> +		rte_ring_free(t->msgq_rsp);
>  	}
>  }
> 
> --
> 2.30.2

Hi Stephen,

The rte_ring_free() and rte_mempool_free() do not state in their API description that freeing a NULL pointer is harmless. Before pushing these changes, please add the necessary note in the API header files for these functions.

In the absence of the clear note in their API description, the user is forced to check for the NULL pointer. I agree that the implementation of these functions does the right think and exits early when the input pointer is NULL, but there is no guarantee that the implementation is not going to change. Agree?

The stdlib free() and the rte_free() do have the clear API description note that freeing a NULL object is harmless, so removing the NULL check before their call is indeed safe.

This stands for all the patches in this set.

Regards,
Cristian
diff mbox series

Patch

diff --git a/examples/pipeline/thread.c b/examples/pipeline/thread.c
index bf11e6144b75..5fe7eae00eb6 100644
--- a/examples/pipeline/thread.c
+++ b/examples/pipeline/thread.c
@@ -89,11 +89,9 @@  thread_free(void)
 			continue;
 
 		/* MSGQs */
-		if (t->msgq_req)
-			rte_ring_free(t->msgq_req);
+		rte_ring_free(t->msgq_req);
 
-		if (t->msgq_rsp)
-			rte_ring_free(t->msgq_rsp);
+		rte_ring_free(t->msgq_rsp);
 	}
 }