Re: [dpdk-dev] [PATCH v5] ethdev: add sanity checks in control APIs

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 4/15/2021 12:09 PM, Min Hu (Connor) wrote:
> This patch adds more sanity checks in control path APIs.
> 
> Fixes: 214ed1acd125 ("ethdev: add iterator to match devargs input")
> Fixes: 3d98f921fbe9 ("ethdev: unify prefix for static functions and variables")
> Fixes: 0366137722a0 ("ethdev: check for invalid device name")
> Fixes: d948f596fee2 ("ethdev: fix port data mismatched in multiple process model")
> Fixes: 5b7ba31148a8 ("ethdev: add port ownership")
> Fixes: f8244c6399d9 ("ethdev: increase port id range")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>

<...>

> @@ -2684,8 +2767,14 @@ rte_eth_stats_get(uint16_t port_id, struct rte_eth_stats *stats)
>   	struct rte_eth_dev *dev;
>   
>   	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> -
>   	dev = &rte_eth_devices[port_id];
> +
> +	if (stats == NULL) {
> +		RTE_ETHDEV_LOG(ERR, "Failed to get ethdev port %u stats by NULL\n",
> +			port_id);
> +		return -EINVAL;
> +	}
> +
>   	memset(stats, 0, sizeof(*stats));
>   
>   	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->stats_get, -ENOTSUP);

Not exactly related to this patch, but related to the API input, some input that 
is filled by API is memset first before passing it to the PMD, which makes 
sense. And for this we have two different ordering with dev_ops check:

1.
memset(input)
check dev_ops, return error if not supported
call dev_ops

2.
check dev_ops, return error if not supported
memset(input)
call dev_ops

Connor, Thomas, Andrew,
Do you think does it have any benifit to unify it, and is one better than other?