On Wed, May 18, 2016 at 5:43 PM, Thomas Monjalon <thomas.monjalon at 6wind.com> wrote:
> 2016-05-18 17:25, Mauricio V?squez: > > On Wed, May 18, 2016 at 5:01 PM, Thomas Monjalon < > thomas.monjalon at 6wind.com> > > wrote: > > > > > 2016-05-18 16:41, Mauricio V?squez: > > > > On Wed, May 18, 2016 at 10:15 AM, Thomas Monjalon < > > > thomas.monjalon at 6wind.com > > > > > wrote: > > > > > > > > > 2016-05-17 22:02, Mauricio V?squez: > > > > > > On Fri, May 13, 2016 at 6:20 PM, Thomas Monjalon < > > > > > thomas.monjalon at 6wind.com> > > > > > > wrote: > > > > > > > 2016-04-29 17:23, Mauricio Vasquez B: > > > > > > > > The RTE_ETH_VALID_PORTID_OR_ERR_RET macro is used in some > places > > > > > > > > to check if a port id is valid or not. This commit makes use > of > > > it in > > > > > > > > some new parts of the code. > > > > > > > > > > > > > > There are other occurences: > > > > > > > rte_eth_dev_socket_id > > > > > > > > > > > > > I missed it. > > > > > > > > > > > > > rte_eth_add_rx_callback > > > > > > > rte_eth_add_tx_callback > > > > > > > rte_eth_remove_rx_callback > > > > > > > rte_eth_remove_tx_callback > > > > > > > > > > > > > The macro can not be used on those ones because they set the > > > rte_errno > > > > > > variable before returning. > > > > > > > > > > It may be a good idea to set rte_errno to EINVAL in these macros. > > > > > > > > > > Generally speaking, rte_errno is not used a lot currently. > > > > > > > > > > > > I noticed that both EINVAL and ENODEV are used. I think that > returning > > > > ENODEV and setting rte_errno to EINVAL would be strange, what do you > > > think > > > > about always using ENODEV? > > > > > > Why EINVAL is used? > > > > > Why not using retval to set errno? > > > > > > > If we do it, the macro could no be used in > > rte_eth_dev_socket_id > > rte_eth_dev_get_device_type > > rte_eth_add_rx_callback > > rte_eth_add_tx_callback > > rte_eth_remove_rx_callback > > rte_eth_remove_tx_callback > > as they do not return an error number. > > So you should not set errno in the existing macro. > But you can create a new macro RTE_ETH_VALID_PORTID_OR_ERRNO_RET > for these functions. > > I realized that RTE_ETH_VALID_PORTID_OR_ERR_RET can be used also in rte_eth_remove_rx_callback and rte_eth_remove_tx_callback. Personally I think it is not worthy to create a macro just for the two missing functions. > > I feel ENODEV would be better but it is an API change, so we should > discuss > > > it later for another patch. > > It looks to be really needed to have an unique kind of error interface to > clean all this mess. >
