> -----Original Message----- > From: Thomas Monjalon [mailto:tho...@monjalon.net] > Sent: Thursday, January 10, 2019 5:35 PM > To: Van Haaren, Harry <harry.van.haa...@intel.com> > Cc: dev@dpdk.org; Pattan, Reshma <reshma.pat...@intel.com>; Dumitrescu, > Cristian <cristian.dumitre...@intel.com>; olivier.m...@6wind.com > Subject: Re: [PATCH] mbuf: fix compile by making sched struct visible > > 10/01/2019 17:50, Harry van Haaren: > > Although C compilation works with the struct rte_mbuf_sched > > declared inside the struct rte_mbuf namespace, C++ fails to > > compile. This fix moves the rte_mbuf_sched struct up to the > > global namespace, instead of declaring it inside the struct > > mbuf namespace. > > > > The struct rte_mbuf_sched is being used on the stack in > > rte_mbuf_sched_get() and as a cast in _set(). For this > > reason, it must be exposed as an available type. > > > > Fixes: 5d3f72100904 ("mbuf: implement generic format for sched field") > > > > Signed-off-by: Harry van Haaren <harry.van.haa...@intel.com> > > > > --- > > > > Cc: reshma.pat...@intel.com > > Cc: cristian.dumitre...@intel.com > > Cc: tho...@monjalon.net > > > > Hey folks, > > > > Currently the mbuf header will fail to compile with a C++ compiler, > > this patch is one possible solution. I'm not particularly happy with > > this as a fix as it reduces mbuf struct readability, however it does > > resolve the issue. > > What are the other possible solutions?
I guess we could avoid using the struct in inline functions, by reading or writing from the mbuf struct directly (without pulling out a temporary rte_mbuf_sched field): get() *queue_id = m->hash.sched.queue_id; ... set() m->hash.sched.queue_id = queue_id; ... I believe this was discussed when the patch was being reviewed; http://patches.dpdk.org/patch/49126/ I have a mild preference to keep all mbuf structs visible inside the unions, it makes it easier to understand the layout of mbuf, hence I prefer the above pseudo code suggestion over the v1 patch sent before. If others have a strong opinion for another solution, I'm ok with it.
