On 12/1/2017 5:17 AM, Bruce Richardson wrote: > On Fri, Dec 01, 2017 at 11:22:12AM +0000, Ananyev, Konstantin wrote: >> >> >>> -----Original Message----- >>> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Bruce Richardson >>> Sent: Friday, December 1, 2017 10:33 AM >>> To: Yigit, Ferruh <ferruh.yi...@intel.com> >>> Cc: Thomas Monjalon <tho...@monjalon.net>; dev@dpdk.org; >>> vl...@cloudius-systems.com >>> Subject: Re: [dpdk-dev] [PATCH 7/7] ethdev: use opaque user callback object >>> >>> On Fri, Dec 01, 2017 at 02:29:57AM +0000, Ferruh Yigit wrote: >>>> "struct rte_eth_rxtx_callback" is defined as internal data structure but >>>> used in public APIs. >>>> >>>> Checking the API documentation shows that intention was using this >>>> object as opaque object. Data structure only used in delete APIs which >>>> doesn't require to know the internals of the data structure. >>>> >>>> Converting callback parameter in API to void pointer should not require >>>> any modification in user application because this data structure was >>>> already marked as internal and only should be used as pointer in >>>> application. >>>> >>>> Signed-off-by: Ferruh Yigit <ferruh.yi...@intel.com> >>>> --- >>> >>> I disagree on this patch. The structure itself is not exposed, only the >>> name, since it is only passed around as a pointer, so there is no need >>> to change the parameters to void pointer. It's a named opaque type. >> >> Personally I think it would be better to do visa-versa: >> make rte_eth_add_(rx|tx)_callback() to return struct rte_eth_rxtx_callback * >> instead of void *. >> Konstantin >> > I didn't realise that it did, so definite +1 to that suggestion.
No issue on having a named opaque type, but unfortunately struct is exposed because of inline functions again. It has been moved into rte_ethdev_core.h but accessible by applications. And since intention is an opaque type, because of "void *" return types, I thought it is better to hide type completely so that application can't access details.
