> -----Original Message----- > From: Stephen Hemminger [mailto:stephen at networkplumber.org] > Sent: Thursday, June 25, 2015 9:44 AM > To: Wang, Liang-min > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v7 1/4] ethdev: add apis to support access > device info > > On Wed, 17 Jun 2015 18:22:12 -0400 > Liang-Min Larry Wang <liang-min.wang at intel.com> wrote: > > > +int > > +rte_eth_dev_reg_length(uint8_t port_id) > > +{ > > + struct rte_eth_dev *dev; > > + > > + if ((dev= &rte_eth_devices[port_id]) == NULL) { > > + PMD_DEBUG_TRACE("Invalid port device\n"); > > + return -ENODEV; > > + } > > Some minor nits: > * for consistency you should add valid port check here. > * style: > - don't do assignment in if() unless it really helps readability > - need whitespace > > if (!rte_eth_dev_is_valid_port(portid)) { > PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id); > return -ENODEV; > } > > dev = &rte_eth_devices[port_id]; > if (dev == NULL) { > PMD_DEBUG("Invalid port device\n"); > return -ENODEV: > } > ... > > This code pattern is so common it really should be a function. > > dev = rte_eth_dev_get(port_id); > if (dev == NULL) { > PMD_DEBUG("Invalid port device\n"); > return -ENODEV; > } > > And then add a macro to generate this??
This is used through-out the rte_ethdev.c, should it be done to the entire file?
