On 10/13/2020 9:55 AM, Thomas Monjalon wrote:
<...>
dev = &rte_eth_devices[port_id];
- RTE_FUNC_PTR_OR_RET(*dev->dev_ops->dev_close);
- (*dev->dev_ops->dev_close)(dev);
+ RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_close, -ENOTSUP);
+ *lasterr = (*dev->dev_ops->dev_close)(dev);
+ if (*lasterr != 0)
+ lasterr = &binerr;
rte_ethdev_trace_close(port_id);
- rte_eth_dev_release_port(dev);
+ *lasterr = rte_eth_dev_release_port(dev);
+
+ return firsterr;
}
This may be personal taste but above error handling looks like unnecessary
complex, what do you think something like below:
close_err = (*dev->dev_ops->dev_close)(dev);
release_err = rte_eth_dev_release_port(dev);
return close_err ? close_err : release_err;
This is what I did first. Then thought about this "elegant" handling.
The pointer solution is just one more line and is more future proof.
I'm fine with any choice. Andrew?
Hm, really hard to make a choice. I tend to choose Thomas's
version. Yes, I agree that it is a bit over-complicated in
this particular case, but thoughts to be future-proof
overweight for me. Plus it adds a pattern on how to handle
such cases in the future.
Yes it's an elegant pattern :)
OK :)
