From: Konstantin Ananyev <konstantin.ananyev@huawei.com>
To: Ariel Otilibili <ariel.otilibili@6wind.com>,
"dev@dpdk.org" <dev@dpdk.org>
Cc: "stable@dpdk.org" <stable@dpdk.org>,
"Thomas Monjalon" <thomas@monjalon.net>,
"David Marchand" <david.marchand@redhat.com>,
"Andrew Rybchenko" <andrew.rybchenko@oktetlabs.ru>,
"Morten Brørup" <mb@smartsharesystems.com>
Subject: RE: [PATCH 2/2] mempool: turn functions into single-exit ones
Date: Sun, 19 Jan 2025 23:44:58 +0000 [thread overview]
Message-ID: <7524c0626a384d458ca64414ca1ff0e4@huawei.com> (raw)
In-Reply-To: <20250119174643.2162110-3-ariel.otilibili@6wind.com>
> Some functions did not set rte_errno; for avoiding that, they are turned
> into single-exit ones.
>
> Bugzilla ID: 1559
> Signed-off-by: Ariel Otilibili <ariel.otilibili@6wind.com>
But reading through public API comments none of these functions are
expected to set rte_errno value.
If rte_mempool_create_empty() forgets to set rte_errno, why it is not
enough just to add missing one in rte_mempool_create_empty()?
> ---
> lib/mempool/rte_mempool_ops.c | 38 ++++++++++++++++++++++++++---------
> 1 file changed, 28 insertions(+), 10 deletions(-)
>
> diff --git a/lib/mempool/rte_mempool_ops.c b/lib/mempool/rte_mempool_ops.c
> index b5c68ac61b67..bf4328645196 100644
> --- a/lib/mempool/rte_mempool_ops.c
> +++ b/lib/mempool/rte_mempool_ops.c
> @@ -33,7 +33,9 @@ rte_mempool_register_ops(const struct rte_mempool_ops *h)
> rte_spinlock_unlock(&rte_mempool_ops_table.sl);
> RTE_MEMPOOL_LOG(ERR,
> "Maximum number of mempool ops structs exceeded");
> - return -ENOSPC;
> + rte_errno = ENOSPC;
> + ops_index = -rte_errno;
> + goto out;
> }
>
> if (h->alloc == NULL || h->enqueue == NULL ||
> @@ -41,7 +43,9 @@ rte_mempool_register_ops(const struct rte_mempool_ops *h)
> rte_spinlock_unlock(&rte_mempool_ops_table.sl);
> RTE_MEMPOOL_LOG(ERR,
> "Missing callback while registering mempool ops");
> - return -EINVAL;
> + rte_errno = -EINVAL;
> + ops_index = -rte_errno;
> + goto out;
> }
>
> if (strlen(h->name) >= sizeof(ops->name) - 1) {
> @@ -49,7 +53,8 @@ rte_mempool_register_ops(const struct rte_mempool_ops *h)
> RTE_MEMPOOL_LOG(DEBUG, "%s(): mempool_ops <%s>: name too long",
> __func__, h->name);
> rte_errno = EEXIST;
> - return -EEXIST;
> + ops_index = -rte_errno;
> + goto out;
> }
>
> ops_index = rte_mempool_ops_table.num_ops++;
> @@ -67,6 +72,7 @@ rte_mempool_register_ops(const struct rte_mempool_ops *h)
>
> rte_spinlock_unlock(&rte_mempool_ops_table.sl);
>
> +out:
> return ops_index;
> }
>
> @@ -151,12 +157,19 @@ rte_mempool_ops_get_info(const struct rte_mempool *mp,
> struct rte_mempool_info *info)
> {
> struct rte_mempool_ops *ops;
> + int ret;
>
> ops = rte_mempool_get_ops(mp->ops_index);
>
> - if (ops->get_info == NULL)
> - return -ENOTSUP;
> - return ops->get_info(mp, info);
> + if (ops->get_info == NULL) {
> + rte_errno = ENOTSUP;
> + ret = -rte_errno;
> + goto out;
> + }
> + ret = ops->get_info(mp, info);
> +
> +out:
> + return ret;
> }
>
>
> @@ -166,12 +179,14 @@ rte_mempool_set_ops_byname(struct rte_mempool *mp, const char *name,
> void *pool_config)
> {
> struct rte_mempool_ops *ops = NULL;
> + int ret = 0;
> unsigned i;
>
> /* too late, the mempool is already populated. */
> if (mp->flags & RTE_MEMPOOL_F_POOL_CREATED) {
> rte_errno = EEXIST;
> - return -rte_errno;
> + ret = -rte_errno;
> + goto out;
> }
>
> for (i = 0; i < rte_mempool_ops_table.num_ops; i++) {
> @@ -182,13 +197,16 @@ rte_mempool_set_ops_byname(struct rte_mempool *mp, const char *name,
> }
> }
>
> - if (ops == NULL) {
> + if (!ops) {
> rte_errno = EINVAL;
> - return -rte_errno;
> + ret = -rte_errno;
> + goto out;
> }
>
> mp->ops_index = i;
> mp->pool_config = pool_config;
> rte_mempool_trace_set_ops_byname(mp, name, pool_config);
> - return 0;
> +
> +out:
> + return ret;
> }
> --
> 2.30.2
next prev parent reply other threads:[~2025-01-19 23:45 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-01-19 17:46 [PATCH 0/2] mempool: add rte_errno, and turn functions into single-exit ones Ariel Otilibili
2025-01-19 17:46 ` [PATCH 1/2] mempool: add rte_errno in rte_mempool_set_ops_byname Ariel Otilibili
2025-01-19 17:46 ` [PATCH 2/2] mempool: turn functions into single-exit ones Ariel Otilibili
2025-01-19 23:44 ` Konstantin Ananyev [this message]
2025-01-20 10:05 ` Ariel Otilibili
2025-01-20 12:21 ` [PATCH v2 0/2] add rte_errno to rte_mempool_create_empty, and refactor Ariel Otilibili
2025-01-20 12:21 ` [PATCH v2 1/2] mempool: fix rte_errno in rte_mempool_create_empty Ariel Otilibili
2025-01-24 6:47 ` fengchengwen
2025-02-07 14:30 ` Konstantin Ananyev
2025-02-12 14:13 ` Thomas Monjalon
2025-01-20 12:21 ` [PATCH v2 2/2] mempool: make rte_mempool_create_empty a single-exit Ariel Otilibili
2025-02-07 14:31 ` Konstantin Ananyev
2025-02-13 11:10 ` Ariel Otilibili
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7524c0626a384d458ca64414ca1ff0e4@huawei.com \
--to=konstantin.ananyev@huawei.com \
--cc=andrew.rybchenko@oktetlabs.ru \
--cc=ariel.otilibili@6wind.com \
--cc=david.marchand@redhat.com \
--cc=dev@dpdk.org \
--cc=mb@smartsharesystems.com \
--cc=stable@dpdk.org \
--cc=thomas@monjalon.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.