-----Original Message-----
From: Stephen Hemminger <stephen@networkplumber.org>
Sent: Wednesday, 29 January 2025 18:25
To: Shani Peretz <redacted>
Cc: dev@dpdk.org; NBU-Contact-Thomas Monjalon (EXTERNAL)
[off-list ref]; Tyler Retzlaff [off-list ref]; Parav
Pandit [off-list ref]; Xueming Li [off-list ref]; Nipun Gupta
[off-list ref]; Nikhil Agarwal [off-list ref]; Hemant
Agrawal [off-list ref]; Sachin Saxena
[off-list ref]; Rosen Xu [off-list ref]; Chenbo Xia
[off-list ref]; Tomasz Duszynski [off-list ref];
Chengwen Feng [off-list ref]; NBU-Contact-longli
(EXTERNAL) [off-list ref]; Wei Hu [off-list ref]; Bruce
Richardson [off-list ref]; Kevin Laatz
[off-list ref]; Jan Blunck [off-list ref]
Subject: Re: [PATCH v4] bus: fix inconsistent representation of PCI numbers

External email: Use caution opening links or attachments


On Wed, 29 Jan 2025 10:54:16 +0200
Shani Peretz [off-list ref] wrote:

quoted

+create_pci_dev(const char *name)
+{
+     int port_id;
+     uint8_t slave_mac1[] = {0x00, 0xFF, 0x00, 0xFF, 0x00, 0x00 };
+     struct rte_ether_addr *mac_addr = (struct rte_ether_addr
+*)slave_mac1;

Use different initializer and you can avoid the need for cast here.

quoted

+/**
+ * General device name comparison. Will compare by using the specific
+bus
+ * compare function or by comparing the names directly.
+ *
+ * @param dev
+ *   Device handle.
+ * @param name
+ *   Name to compare against.
+ * @return
+ *   0 if the device matches the name. Nonzero otherwise.
+ */
+__rte_internal
+int rte_cmp_dev_name(const struct rte_device *dev, const void *name);

It would make more sense to me if name was a character not void pointer.

The design might be clearer if bus address was more of an typedef with a
pointer and size together. Treat it more like an object.

Okay so just to understand,
this is the struct I am adding:

	struct rte_bus_address {
	       const void *addr;
	       size_t size;
	};

This is how I pass it to find_device:
	
	struct rte_bus_address dev_addr = {
	          .addr = da->name,
	          .size = RTE_DEV_NAME_MAX_LEN
	};
	dev = da->bus->find_device(NULL, rte_cmp_dev_name, &dev_addr);

And this is how I use it in rte_cmp_dev_name:

	rte_cmp_dev_name(const struct rte_device *dev1, const void *addr)
	 {
	       const struct rte_bus_address *dev2_addr = addr;
	…
	}

Is that what you meant? 
Also, I'm not sure if the size is really needed, because we check the size of the parsed name, which may be different than the size of the original name