diff mbox series

[v2,2/2] mempool: make rte_mempool_create_empty a single-exit

Message ID 20250120122156.2480524-3-ariel.otilibili@6wind.com (mailing list archive)
State New
Delegated to: Thomas Monjalon
Headers
Series add rte_errno to rte_mempool_create_empty, and refactor |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/github-robot: build success github build: passed
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-marvell-Functional success Functional Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-unit-amd64-testing success Testing PASS
ci/iol-sample-apps-testing success Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/intel-Functional success Functional PASS
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-compile-amd64-testing success Testing PASS

Commit Message

Ariel Otilibili Jan. 20, 2025, 12:21 p.m. UTC 
For properly setting rte_errno, and exiting rte_mempool_create_empty()
from a single place.

Bugzilla ID: 1559
Signed-off-by: Ariel Otilibili <ariel.otilibili@6wind.com>
---
 lib/mempool/rte_mempool.c | 27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)

Comments

Konstantin Ananyev Feb. 7, 2025, 2:31 p.m. UTC | #1 
> For properly setting rte_errno, and exiting rte_mempool_create_empty()
> from a single place.
> 
> Bugzilla ID: 1559
> Signed-off-by: Ariel Otilibili <ariel.otilibili@6wind.com>
> ---
>  lib/mempool/rte_mempool.c | 27 +++++++++++++++------------
>  1 file changed, 15 insertions(+), 12 deletions(-)
> 
> diff --git a/lib/mempool/rte_mempool.c b/lib/mempool/rte_mempool.c
> index 1e4f24783c0b..3f9a4a1771d3 100644
> --- a/lib/mempool/rte_mempool.c
> +++ b/lib/mempool/rte_mempool.c
> @@ -831,20 +831,20 @@ rte_mempool_create_empty(const char *name, unsigned n, unsigned elt_size,
>  	/* asked for zero items */
>  	if (n == 0) {
>  		rte_errno = EINVAL;
> -		return NULL;
> +		goto exit;
>  	}
> 
>  	/* asked cache too big */
>  	if (cache_size > RTE_MEMPOOL_CACHE_MAX_SIZE ||
>  	    CALC_CACHE_FLUSHTHRESH(cache_size) > n) {
>  		rte_errno = EINVAL;
> -		return NULL;
> +		goto exit;
>  	}
> 
>  	/* enforce only user flags are passed by the application */
>  	if ((flags & ~RTE_MEMPOOL_VALID_USER_FLAGS) != 0) {
>  		rte_errno = EINVAL;
> -		return NULL;
> +		goto exit;
>  	}
> 
>  	/*
> @@ -860,7 +860,7 @@ rte_mempool_create_empty(const char *name, unsigned n, unsigned elt_size,
>  	/* calculate mempool object sizes. */
>  	if (!rte_mempool_calc_obj_size(elt_size, flags, &objsz)) {
>  		rte_errno = EINVAL;
> -		return NULL;
> +		goto exit;
>  	}
> 
>  	rte_mcfg_mempool_write_lock();
> @@ -877,7 +877,7 @@ rte_mempool_create_empty(const char *name, unsigned n, unsigned elt_size,
>  	te = rte_zmalloc("MEMPOOL_TAILQ_ENTRY", sizeof(*te), 0);
>  	if (te == NULL) {
>  		RTE_MEMPOOL_LOG(ERR, "Cannot allocate tailq entry!");
> -		goto exit_unlock;
> +		goto unlock;
>  	}
> 
>  	mempool_size = RTE_MEMPOOL_HEADER_SIZE(mp, cache_size);
> @@ -887,12 +887,12 @@ rte_mempool_create_empty(const char *name, unsigned n, unsigned elt_size,
>  	ret = snprintf(mz_name, sizeof(mz_name), RTE_MEMPOOL_MZ_FORMAT, name);
>  	if (ret < 0 || ret >= (int)sizeof(mz_name)) {
>  		rte_errno = ENAMETOOLONG;
> -		goto exit_unlock;
> +		goto unlock;
>  	}
> 
>  	mz = rte_memzone_reserve(mz_name, mempool_size, socket_id, mz_flags);
>  	if (mz == NULL)
> -		goto exit_unlock;
> +		goto unlock;
> 
>  	/* init the mempool structure */
>  	mp = mz->addr;
> @@ -900,7 +900,7 @@ rte_mempool_create_empty(const char *name, unsigned n, unsigned elt_size,
>  	ret = strlcpy(mp->name, name, sizeof(mp->name));
>  	if (ret < 0 || ret >= (int)sizeof(mp->name)) {
>  		rte_errno = ENAMETOOLONG;
> -		goto exit_unlock;
> +		goto unlock;
>  	}
>  	mp->mz = mz;
>  	mp->size = n;
> @@ -930,7 +930,7 @@ rte_mempool_create_empty(const char *name, unsigned n, unsigned elt_size,
> 
>  	if (ret) {
>  		rte_errno = -ret;
> -		goto exit_unlock;
> +		goto unlock;
>  	}
> 
>  	/*
> @@ -956,13 +956,16 @@ rte_mempool_create_empty(const char *name, unsigned n, unsigned elt_size,
> 
>  	rte_mempool_trace_create_empty(name, n, elt_size, cache_size,
>  		private_data_size, flags, mp);
> -	return mp;
> +	goto exit;
> 
> -exit_unlock:
> +unlock:
>  	rte_mcfg_mempool_write_unlock();
>  	rte_free(te);
>  	rte_mempool_free(mp);
> -	return NULL;
> +	mp = NULL;
> +
> +exit:
> +	return mp;
>  }
> 
>  /* create the mempool */
> --

Personally, I don't see much profit in such refactoring.

> 2.30.2
> <
Ariel Otilibili Feb. 13, 2025, 11:10 a.m. UTC | #2 
Hi Konstantin,

On Fri, Feb 7, 2025 at 3:31 PM Konstantin Ananyev <
konstantin.ananyev@huawei.com> wrote:

> Personally, I don't see much profit in such refactoring.
>

Then, please drop this patch. I gave room for other people to react, none
came up.

Thanks for your feedback,
Ariel

>
> > 2.30.2
> > <
>
diff mbox series

Patch

diff --git a/lib/mempool/rte_mempool.c b/lib/mempool/rte_mempool.c
index 1e4f24783c0b..3f9a4a1771d3 100644
--- a/lib/mempool/rte_mempool.c
+++ b/lib/mempool/rte_mempool.c
@@ -831,20 +831,20 @@  rte_mempool_create_empty(const char *name, unsigned n, unsigned elt_size,
 	/* asked for zero items */
 	if (n == 0) {
 		rte_errno = EINVAL;
-		return NULL;
+		goto exit;
 	}
 
 	/* asked cache too big */
 	if (cache_size > RTE_MEMPOOL_CACHE_MAX_SIZE ||
 	    CALC_CACHE_FLUSHTHRESH(cache_size) > n) {
 		rte_errno = EINVAL;
-		return NULL;
+		goto exit;
 	}
 
 	/* enforce only user flags are passed by the application */
 	if ((flags & ~RTE_MEMPOOL_VALID_USER_FLAGS) != 0) {
 		rte_errno = EINVAL;
-		return NULL;
+		goto exit;
 	}
 
 	/*
@@ -860,7 +860,7 @@  rte_mempool_create_empty(const char *name, unsigned n, unsigned elt_size,
 	/* calculate mempool object sizes. */
 	if (!rte_mempool_calc_obj_size(elt_size, flags, &objsz)) {
 		rte_errno = EINVAL;
-		return NULL;
+		goto exit;
 	}
 
 	rte_mcfg_mempool_write_lock();
@@ -877,7 +877,7 @@  rte_mempool_create_empty(const char *name, unsigned n, unsigned elt_size,
 	te = rte_zmalloc("MEMPOOL_TAILQ_ENTRY", sizeof(*te), 0);
 	if (te == NULL) {
 		RTE_MEMPOOL_LOG(ERR, "Cannot allocate tailq entry!");
-		goto exit_unlock;
+		goto unlock;
 	}
 
 	mempool_size = RTE_MEMPOOL_HEADER_SIZE(mp, cache_size);
@@ -887,12 +887,12 @@  rte_mempool_create_empty(const char *name, unsigned n, unsigned elt_size,
 	ret = snprintf(mz_name, sizeof(mz_name), RTE_MEMPOOL_MZ_FORMAT, name);
 	if (ret < 0 || ret >= (int)sizeof(mz_name)) {
 		rte_errno = ENAMETOOLONG;
-		goto exit_unlock;
+		goto unlock;
 	}
 
 	mz = rte_memzone_reserve(mz_name, mempool_size, socket_id, mz_flags);
 	if (mz == NULL)
-		goto exit_unlock;
+		goto unlock;
 
 	/* init the mempool structure */
 	mp = mz->addr;
@@ -900,7 +900,7 @@  rte_mempool_create_empty(const char *name, unsigned n, unsigned elt_size,
 	ret = strlcpy(mp->name, name, sizeof(mp->name));
 	if (ret < 0 || ret >= (int)sizeof(mp->name)) {
 		rte_errno = ENAMETOOLONG;
-		goto exit_unlock;
+		goto unlock;
 	}
 	mp->mz = mz;
 	mp->size = n;
@@ -930,7 +930,7 @@  rte_mempool_create_empty(const char *name, unsigned n, unsigned elt_size,
 
 	if (ret) {
 		rte_errno = -ret;
-		goto exit_unlock;
+		goto unlock;
 	}
 
 	/*
@@ -956,13 +956,16 @@  rte_mempool_create_empty(const char *name, unsigned n, unsigned elt_size,
 
 	rte_mempool_trace_create_empty(name, n, elt_size, cache_size,
 		private_data_size, flags, mp);
-	return mp;
+	goto exit;
 
-exit_unlock:
+unlock:
 	rte_mcfg_mempool_write_unlock();
 	rte_free(te);
 	rte_mempool_free(mp);
-	return NULL;
+	mp = NULL;
+
+exit:
+	return mp;
 }
 
 /* create the mempool */