Re: [PATCH v8 1/5] ethdev: add apis to support access device info

-----Original Message-----
From: Stephen Hemminger [mailto:stephen@networkplumber.org]
Sent: Friday, June 26, 2015 12:52 PM
To: Wang, Liang-min
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v8 1/5] ethdev: add apis to support access
device info

On Fri, 26 Jun 2015 10:26:43 -0400
Liang-Min Larry Wang [off-list ref] wrote:

quoted

add new apis:
- rte_eth_dev_default_mac_addr_set
- rte_eth_dev_reg_length
- rte_eth_dev_reg_info
- rte_eth_dev_eeprom_length
- rte_eth_dev_get_eeprom
- rte_eth_dev_set_eeprom

to enable reading device parameters (mac-addr, register,
eeprom) based upon ethtool alike
data parameter specification.

Signed-off-by: Liang-Min Larry Wang <redacted>

I agree in principal, but has lots of style issues (see report from checkpatch).

ERROR: spaces required around that '=' (ctx:VxW)
#193: FILE: lib/librte_ether/rte_ethdev.c:3677:
+	if ((dev= &rte_eth_devices[port_id]) == NULL) {
 	        ^

ERROR: do not use assignment in if condition
#193: FILE: lib/librte_ether/rte_ethdev.c:3677:
+	if ((dev= &rte_eth_devices[port_id]) == NULL) {

ERROR: spaces required around that '=' (ctx:VxW)
#213: FILE: lib/librte_ether/rte_ethdev.c:3697:
+	if ((dev= &rte_eth_devices[port_id]) == NULL) {
 	        ^

ERROR: do not use assignment in if condition
#213: FILE: lib/librte_ether/rte_ethdev.c:3697:
+	if ((dev= &rte_eth_devices[port_id]) == NULL) {

ERROR: spaces required around that '=' (ctx:VxW)
#232: FILE: lib/librte_ether/rte_ethdev.c:3716:
+	if ((dev= &rte_eth_devices[port_id]) == NULL) {
 	        ^

ERROR: do not use assignment in if condition
#232: FILE: lib/librte_ether/rte_ethdev.c:3716:
+	if ((dev= &rte_eth_devices[port_id]) == NULL) {

ERROR: spaces required around that '=' (ctx:VxW)
#251: FILE: lib/librte_ether/rte_ethdev.c:3735:
+	if ((dev= &rte_eth_devices[port_id]) == NULL) {
 	        ^

ERROR: do not use assignment in if condition
#251: FILE: lib/librte_ether/rte_ethdev.c:3735:
+	if ((dev= &rte_eth_devices[port_id]) == NULL) {

ERROR: spaces required around that '=' (ctx:VxW)
#270: FILE: lib/librte_ether/rte_ethdev.c:3754:
+	if ((dev= &rte_eth_devices[port_id]) == NULL) {
 	        ^

ERROR: do not use assignment in if condition
#270: FILE: lib/librte_ether/rte_ethdev.c:3754:
+	if ((dev= &rte_eth_devices[port_id]) == NULL) {

This part of code is the same as coding in other part of the same file such as:

int
rte_eth_dev_bypass_wd_timeout_show(uint8_t port_id, uint32_t *wd_timeout)
{
	struct rte_eth_dev *dev;

	if (!rte_eth_dev_is_valid_port(port_id)) {
		PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id);
		return -ENODEV;
	}

	if ((dev= &rte_eth_devices[port_id]) == NULL) {
		PMD_DEBUG_TRACE("Invalid port device\n");
		return -ENODEV;
	}

	FUNC_PTR_OR_ERR_RET(*dev->dev_ops->bypass_wd_timeout_show, -ENOTSUP);
	(*dev->dev_ops->bypass_wd_timeout_show)(dev, wd_timeout);
	return 0;
}
I would suggest that someone (either you or me) to do a code cleaning after all patches for DPDK 2.1 have been decided. Does that sound good to you?