[dpdk-dev] [PATCH 7/7] ethdev: use opaque user callback object

[Ferruh Yigit]

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-bounces at dpdk.org] On Behalf Of Bruce Richardson
>>> Sent: Friday, December 1, 2017 10:33 AM
>>> To: Yigit, Ferruh <ferruh.yigit at intel.com>
>>> Cc: Thomas Monjalon <thomas at monjalon.net>; dev at dpdk.org; vladz at 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.yigit at 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.