> -----Original Message----- > From: Stephen Hemminger [mailto:stephen at networkplumber.org] > Sent: Friday, June 26, 2015 12:52 PM > To: Wang, Liang-min > Cc: dev at 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 <liang-min.wang at intel.com> wrote: > > > 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 <liang-min.wang at intel.com> > > 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?