RE: [PATCH 4/6] mempool: various cleanups

[Konstantin Ananyev <konstantin.ananyev@huawei.com>]

> Removed audit call from dump function, where it doesn't belong.
> 
> Added malloc/dealloc attribute to private cache alloc/free functions.
> 
> Improved checks in mempool ops registration function.
> 
> Improved cache audit checks.
> 
> Added detailed description to definition of mempool name length.
> 
> Updated some comments, and moved around some misplaced comments.
> 
> Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
> ---

....

> diff --git a/lib/mempool/rte_mempool_ops.c
> b/lib/mempool/rte_mempool_ops.c
> index 066bec36fc..6d99c7a25c 100644
> --- a/lib/mempool/rte_mempool_ops.c
> +++ b/lib/mempool/rte_mempool_ops.c
> @@ -27,7 +27,7 @@ int
>  rte_mempool_register_ops(const struct rte_mempool_ops *h)
>  {
>  	struct rte_mempool_ops *ops;
> -	int16_t ops_index;
> +	unsigned int ops_index;
> 
>  	rte_spinlock_lock(&rte_mempool_ops_table.sl);
> 
> @@ -47,12 +47,22 @@ rte_mempool_register_ops(const struct
> rte_mempool_ops *h)
>  		return -EINVAL;
>  	}
> 
> -	if (strlen(h->name) >= sizeof(ops->name) - 1) {
> +	if (strnlen(h->name, sizeof(h->name)) > sizeof(ops->name) - 1) {
>  		rte_spinlock_unlock(&rte_mempool_ops_table.sl);
> -		RTE_MEMPOOL_LOG(DEBUG, "%s(): mempool_ops <%s>: name
> too long",
> -				__func__, h->name);
> -		rte_errno = EEXIST;
> -		return -EEXIST;
> +		char name[sizeof(h->name) + 1];
> +		strlcpy(name, h->name, sizeof(name));

Curious, why do you need to copy?
Why can't t->name be used directly withiin the LOG()?

> +		RTE_MEMPOOL_LOG(ERR, "%s(): mempool_ops <%s>: name too
> long",
> +				__func__, name);
> +		return -ENAMETOOLONG;
> +	}
> +
> +	for (ops_index = 0; ops_index < rte_mempool_ops_table.num_ops;
> ops_index++) {
> +		if (!strcmp(h->name,
> rte_mempool_ops_table.ops[ops_index].name)) {
> +			rte_spinlock_unlock(&rte_mempool_ops_table.sl);
> +			RTE_MEMPOOL_LOG(ERR, "%s(): mempool_ops <%s>:
> name exists",
> +					__func__, h->name);
> +			return -EEXIST;
> +		}
>  	}
> 
>  	ops_index = rte_mempool_ops_table.num_ops++;
> @@ -70,6 +80,8 @@ rte_mempool_register_ops(const struct rte_mempool_ops
> *h)
> 
>  	rte_spinlock_unlock(&rte_mempool_ops_table.sl);
> 
> +	RTE_MEMPOOL_LOG(DEBUG, "Registered mempool ops <%s> at index
> %u", ops->name, ops_index);
> +
>  	return ops_index;
>  }
> 
> @@ -185,8 +197,12 @@ rte_mempool_set_ops_byname(struct rte_mempool
> *mp, const char *name,
>  		}
>  	}
> 
> -	if (ops == NULL)
> +	if (ops == NULL) {
> +		RTE_MEMPOOL_LOG(DEBUG,
> +				"Cannot set unknown mempool ops <%s>, of %u
> ops registered",
> +				name, i);
>  		return -EINVAL;
> +	}
> 
>  	mp->ops_index = i;
>  	mp->pool_config = pool_config;
> --
> 2.43.0