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

[Wang, Liang-min]

> -----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?