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