lib/ethdev: sanity check on callback before resource allocation

Message ID 20230711011809.96677-1-squirrel.prog@gmail.com (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers
Series lib/ethdev: sanity check on callback before resource allocation |

Checks

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

Commit Message

Kaiyu Zhang July 11, 2023, 1:18 a.m. UTC
  sanity check is performed after a rte_eth_dev object is allocated.
The object is not freed if the check fails, though in the current
reality this never happens, but it's better programming paradigm
to move the quick check up front to the start of
rte_eth_dev_pci_generic_probe.

Signed-off-by: Kaiyu Zhang <squirrel.prog@gmail.com>
---
 lib/ethdev/ethdev_pci.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
  

Comments

fengchengwen July 11, 2023, 1:48 a.m. UTC | #1
Please add Cc.
Reviewed-by: Chengwen Feng <fengchengwen@huawei.com>

On 2023/7/11 9:18, Kaiyu Zhang wrote:
> sanity check is performed after a rte_eth_dev object is allocated.
> The object is not freed if the check fails, though in the current
> reality this never happens, but it's better programming paradigm
> to move the quick check up front to the start of
> rte_eth_dev_pci_generic_probe.
> 
> Signed-off-by: Kaiyu Zhang <squirrel.prog@gmail.com>
> ---
>  lib/ethdev/ethdev_pci.h | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/ethdev/ethdev_pci.h b/lib/ethdev/ethdev_pci.h
> index 94b8fba5d7..320e3e0093 100644
> --- a/lib/ethdev/ethdev_pci.h
> +++ b/lib/ethdev/ethdev_pci.h
> @@ -126,12 +126,13 @@ rte_eth_dev_pci_generic_probe(struct rte_pci_device *pci_dev,
>  	struct rte_eth_dev *eth_dev;
>  	int ret;
>  
> +	if (*dev_init == NULL)
> +		return -EINVAL;
> +
>  	eth_dev = rte_eth_dev_pci_allocate(pci_dev, private_data_size);
>  	if (!eth_dev)
>  		return -ENOMEM;
>  
> -	if (*dev_init == NULL)
> -		return -EINVAL;
>  	ret = dev_init(eth_dev);
>  	if (ret)
>  		rte_eth_dev_release_port(eth_dev);
>
  
Ferruh Yigit July 11, 2023, 1:37 p.m. UTC | #2
On 7/11/2023 2:48 AM, fengchengwen wrote:

> On 2023/7/11 9:18, Kaiyu Zhang wrote:
>> sanity check is performed after a rte_eth_dev object is allocated.
>> The object is not freed if the check fails, though in the current
>> reality this never happens, but it's better programming paradigm
>> to move the quick check up front to the start of
>> rte_eth_dev_pci_generic_probe.
>>
>> Signed-off-by: Kaiyu Zhang <squirrel.prog@gmail.com>
>> 
> 
> Reviewed-by: Chengwen Feng <fengchengwen@huawei.com>
> 
    Fixes: dcd5c8112bc3 ("ethdev: add PCI driver helpers")
    Cc: stable@dpdk.org

Reviewed-by: Ferruh Yigit <ferruh.yigit@amd.com>

Applied to dpdk-next-net/main, thanks.
  
Stephen Hemminger Aug. 1, 2023, 4:19 p.m. UTC | #3
On Tue, 11 Jul 2023 01:18:09 +0000
Kaiyu Zhang <squirrel.prog@gmail.com> wrote:

> sanity check is performed after a rte_eth_dev object is allocated.
> The object is not freed if the check fails, though in the current
> reality this never happens, but it's better programming paradigm
> to move the quick check up front to the start of
> rte_eth_dev_pci_generic_probe.
> 
> Signed-off-by: Kaiyu Zhang <squirrel.prog@gmail.com>

The term sanity check is on the non-inclusive naming word list.
Please don't use that term in DPDK.
There is an outstanding patchset to remove all use of the term.
  

Patch

diff --git a/lib/ethdev/ethdev_pci.h b/lib/ethdev/ethdev_pci.h
index 94b8fba5d7..320e3e0093 100644
--- a/lib/ethdev/ethdev_pci.h
+++ b/lib/ethdev/ethdev_pci.h
@@ -126,12 +126,13 @@  rte_eth_dev_pci_generic_probe(struct rte_pci_device *pci_dev,
 	struct rte_eth_dev *eth_dev;
 	int ret;
 
+	if (*dev_init == NULL)
+		return -EINVAL;
+
 	eth_dev = rte_eth_dev_pci_allocate(pci_dev, private_data_size);
 	if (!eth_dev)
 		return -ENOMEM;
 
-	if (*dev_init == NULL)
-		return -EINVAL;
 	ret = dev_init(eth_dev);
 	if (ret)
 		rte_eth_dev_release_port(eth_dev);