On Tue, Dec 19, 2017 at 09:51:10PM +0100, Thomas Monjalon wrote: > 19/12/2017 18:24, Matan Azrad: > > HI > > > > > -----Original Message----- > > > From: Stephen Hemminger [mailto:step...@networkplumber.org] > > > Sent: Tuesday, December 19, 2017 7:20 PM > > > To: Matan Azrad <ma...@mellanox.com> > > > Cc: Adrien Mazarguil <adrien.mazarg...@6wind.com>; Thomas Monjalon > > > <tho...@monjalon.net>; Gaetan Rivet <gaetan.ri...@6wind.com>; > > > dev@dpdk.org > > > Subject: Re: [dpdk-dev] [PATCH v3 1/6] ethdev: add devop to check removal > > > status > > > > > > On Tue, 19 Dec 2017 17:10:10 +0000 > > > Matan Azrad <ma...@mellanox.com> wrote: > > > > > > > int > > > > +rte_eth_dev_is_removed(uint16_t port_id) { > > > > + struct rte_eth_dev *dev; > > > > + int ret; > > > > + > > > > + RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, 0); > > > > + > > > > + dev = &rte_eth_devices[port_id]; > > > > + > > > > + RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->is_removed, 0); > > > > + > > > > + if (dev->state == RTE_ETH_DEV_REMOVED) > > > > + return 1; > > > > + > > > > + ret = dev->dev_ops->is_removed(dev); > > > > + if (ret != 0) > > > > + dev->state = RTE_ETH_DEV_REMOVED; > > > > + > > > > + return ret; > > > > +} > > > > + > > > > > > This looks good. > > > May be a candidate to use bool instead of int for return value? > > > > Yes, I thought about it but didn't see any precedence for bool usage in > > ethdev APIs. > > Guys, what do you think? > > I think this function can return error, isn't it? > (look at macros *_OR_ERR_RET used in the function) >
But those macros are used to return 0. While I think I see a logic behind it, I think it is surprising the API user, which is not ideal. -- Gaëtan Rivet 6WIND
