Skip to contentSkip to conversation

lib/ethdev: sanity check on callback before resource allocation

PATCH Applied Kaiyu Zhang 4 participants 4 messages Jul 11, 2023 – Aug 1, 2023

lore
Share & tools
lore

Kaiyu Zhang

author
#0 lore raw

PATCH 1/1 lib/ethdev: sanity check on callback before resource allocation

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 <[email protected]>
---
 lib/ethdev/ethdev_pci.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
lib/ethdev/ethdev_pci.h +3 -2
index 94b8fba5d7..320e3e0093 100644
@@ -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);
Signature · 1 line
2.34.1
Reviewed-by on v1 1/1
#1 lore raw

Please add Cc.

Reviewed-by Chengwen Feng <[email protected]>
Quoted 32 lines · Kaiyu Zhang

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 <[email protected]>
---
 lib/ethdev/ethdev_pci.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
lib/ethdev/ethdev_pci.h +3 -2
index 94b8fba5d7..320e3e0093 100644
@@ -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);
Reviewed-by Applied
#2 lore raw
Quoted 7 lines · fengchengwen
Quoted 6 lines · Kaiyu Zhang

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.

Quoted 5 lines
Quoted 2 lines
Signed-off-by Kaiyu Zhang <[email protected]>
Reviewed-by Chengwen Feng <[email protected]>
Fixes dcd5c8112bc3 ("ethdev: add PCI driver helpers")
Cc [email protected]
Reviewed-by Ferruh Yigit <[email protected]>

Applied to dpdk-next-net/main, thanks.

#3 lore raw
In reply to Kaiyu Zhang
Quoted 7 lines · On Tue, 11 Jul 2023 01:18:09 +0000 Kaiyu Zhang <[email protected]> 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 <[email protected]>

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.