[dpdk-dev] [PATCH v4] ethdev: return named opaque type instead of void pointer
Neil Horman
nhorman at tuxdriver.com
Fri Mar 9 16:16:16 CET 2018
On Fri, Mar 09, 2018 at 01:00:35PM +0000, Ferruh Yigit wrote:
> On 3/9/2018 12:36 PM, Neil Horman wrote:
> > On Fri, Mar 09, 2018 at 11:25:31AM +0000, Ferruh Yigit wrote:
> >> "struct rte_eth_rxtx_callback" is defined as internal data structure and
> >> used as named opaque type.
> >>
> >> So the functions that are adding callbacks can return objects in this
> >> type instead of void pointer.
> >>
> >> Signed-off-by: Ferruh Yigit <ferruh.yigit at intel.com>
> >> Acked-by: Stephen Hemminger <stephen at networkplumber.org>
> >> ---
> >> v2:
> >> * keep using struct * in parameters, instead add callback functions
> >> return struct rte_eth_rxtx_callback pointer.
> >>
> >> v4:
> >> * Remove deprecation notice. LIBABIVER already increased in this release
> >> ---
> >> doc/guides/rel_notes/deprecation.rst | 7 -------
> >> lib/librte_ether/rte_ethdev.c | 6 +++---
> >> lib/librte_ether/rte_ethdev.h | 13 ++++++++-----
> >> 3 files changed, 11 insertions(+), 15 deletions(-)
> >>
> > This doesn't quite make sense to me. If rte_eth_rxtx_callback is defined as an
> > internal data structure, then it shouldn't be used as part of the prototype for
> > an exported function, as the structure will then no longer be a internal data
> > structure, but rather part of the public ABI.
>
> "struct rte_eth_rxtx_callback" is internal data structure. And application
> should not access elements of this structure.
>
> "struct rte_eth_rxtx_callback;" is defined in the public header, so applications
> can use it as opaque type.
>
> It is possible that both "add" and "remove" APIs use "void *" and API itself can
> cast it. But the inconsistency was "add" related APIs return "void *" and
> "remove" related APIs require a parameter in "struct rte_eth_rxtx_callback *" type.
>
> While unifying the usage, "struct rte_eth_rxtx_callback *" preferred against
> "void *", because named opaque type documents intention/usage better.
>
> Thanks,
> ferruh
>
I get what you're saying about rte_eth_rxtx_callback being an internals
structure (or its intent is to be an internal structure), but it doesn't seem to
hold up to the header file layout. rte_eth_rxtx_callback is defined in
rte_ethdev_core.h which according to the makefile, is listed as a symlinked
file, and therefore available for external applications to include. This
negates the intended opaque nature of the struct. I think before you do this,
you want to rectify that.
Neil
> >
> > Neil
> >
>
>
More information about the dev
mailing list