> From: David Marchand [mailto:david.march...@redhat.com] > Sent: Wednesday, 26 October 2022 11.21 > > On Wed, Oct 26, 2022 at 11:05 AM Morten Brørup > <m...@smartsharesystems.com> wrote: > > > > > From: David Marchand [mailto:david.march...@redhat.com] > > > Sent: Wednesday, 14 September 2022 09.58 > > > > > > Those macros have no real value and are easily replaced with a > simple > > > if() block. > > > > > > Existing users have been converted using a new cocci script. > > > Deprecate them. > > > > > > Signed-off-by: David Marchand <david.march...@redhat.com> > > > --- > > > > [...] > > > > > /* Macros to check for invalid function pointers */ > > > -#define RTE_FUNC_PTR_OR_ERR_RET(func, retval) do { \ > > > +#define RTE_FUNC_PTR_OR_ERR_RET(func, retval) > > > RTE_DEPRECATED(RTE_FUNC_PTR_OR_ERR_RET) \ > > > +do { \ > > > if ((func) == NULL) \ > > > return retval; \ > > > } while (0) > > > > > > -#define RTE_FUNC_PTR_OR_RET(func) do { \ > > > +#define RTE_FUNC_PTR_OR_RET(func) > RTE_DEPRECATED(RTE_FUNC_PTR_OR_RET) > > > \ > > > +do { \ > > > if ((func) == NULL) \ > > > return; \ > > > } while (0) > > > > rte_ethdev.h has somewhat similar macros [1]: > > > > /* Macros to check for valid port */ > > #define RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, retval) do { \ > > if (!rte_eth_dev_is_valid_port(port_id)) { \ > > RTE_ETHDEV_LOG(ERR, "Invalid port_id=%u\n", port_id); > \ > > return retval; \ > > } \ > > } while (0) > > > > #define RTE_ETH_VALID_PORTID_OR_RET(port_id) do { \ > > if (!rte_eth_dev_is_valid_port(port_id)) { \ > > RTE_ETHDEV_LOG(ERR, "Invalid port_id=%u\n", port_id); > \ > > return; \ > > } \ > > } while (0) > > > > However, these log an error message. It makes me wonder about > consistency... > > > > Are the RTE_FUNC_PTR_OR_* macros a special case, or should similar > macros, such as RTE_ETH_VALID_PORTID_OR_*, also be deprecated? > > > > Or should the RTE_FUNC_PTR_OR_* macros be modified to log an error > message instead of being deprecated? > > The difference is that those ethdev macros validate something > expressed in their name "is this portid valid". > > The RTE_FUNC_PTR_OR_* macros were really just a if() + return block. > I don't see what message to log for them.
The RTE_ETH_VALID_PORTID_OR_* macros seem to serve the same purpose as the RTE_FUNC_PTR_OR_* macros. Is it just a coincidence that the RTE_FUNC_PTR_OR_* macros don't log an error message, while similar macros do? Cleaning up is certainly good, so I don't object to this patch! I'm only asking if a broader scope applies: Are the RTE_FUNC_PTR_OR_* macros a unique case (so they can be removed as suggested), or should we add an error message to the macros instead of removing them?
