Re: [PATCH 5/5] eventdev: fix compilation with clang C++ builds

[Bruce Richardson <bruce.richardson@intel.com>]

On Fri, Mar 11, 2022 at 05:11:02PM -0800, Stephen Hemminger wrote:
> On Fri, 11 Mar 2022 20:05:23 +0000
> Bruce Richardson <bruce.richardson@intel.com> wrote:
> 
> > When compiling on FreeBSD with clang and include checking enabled,
> > errors are emitted due to differences in how empty structs/unions are
> > handled in C and C++, as C++ structs cannot have zero size.
> > 
> > ../lib/eventdev/rte_eventdev.h:992:2: error: union has size 0 in C, non-zero size in C++
> > 
> > Since the contents of the union are all themselves of zero size,
> > the actual union wrapper is unnecessary. We therefore remove it for C++
> > builds - though keep it for C builds for safety and clarity of
> > understanding the code. The alignment constraint on the union is
> > unnecessary in the case where the whole struct is aligned on a 16-byte
> > boundary, so we add that constraint to the overall structure to ensure
> > it applies for C++ code as well as C.
> > 
> > Fixes: 1cc44d409271 ("eventdev: introduce event vector capability")
> > Cc: pbhagavatula@marvell.com
> > 
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > ---
> >  lib/eventdev/rte_eventdev.h | 12 +++++++++++-
> >  1 file changed, 11 insertions(+), 1 deletion(-)
> > 
> > diff --git a/lib/eventdev/rte_eventdev.h b/lib/eventdev/rte_eventdev.h
> > index 67c4a5e036..42a5660169 100644
> > --- a/lib/eventdev/rte_eventdev.h
> > +++ b/lib/eventdev/rte_eventdev.h
> > @@ -984,21 +984,31 @@ struct rte_event_vector {
> >  	};
> >  	/**< Union to hold common attributes of the vector array. */
> >  	uint64_t impl_opaque;
> > +
> > +/* empty structures do not have zero size in C++ leading to compilation errors
> > + * with clang about structure having different sizes in C and C++.
> > + * Since these are all zero-sized arrays, we can omit the "union" wrapper for
> > + * C++ builds, removing the warning.
> > + */
> > +#ifndef __cplusplus
> >  	/**< Implementation specific opaque value.
> >  	 * An implementation may use this field to hold implementation specific
> >  	 * value to share between dequeue and enqueue operation.
> >  	 * The application should not modify this field.
> >  	 */
> >  	union {
> > +#endif
> >  		struct rte_mbuf *mbufs[0];
> >  		void *ptrs[0];
> >  		uint64_t *u64s[0];
> > +#ifndef __cplusplus
> >  	} __rte_aligned(16);
> > +#endif
> >  	/**< Start of the vector array union. Depending upon the event type the
> >  	 * vector array can be an array of mbufs or pointers or opaque u64
> >  	 * values.
> >  	 */
> > -};
> > +} __rte_aligned(16);
> >  
> >  /* Scheduler type definitions */
> >  #define RTE_SCHED_TYPE_ORDERED          0
> 
> Zero size arrays should be replaced by flexible arrays.
> Linux has this coccinelle script for that:
>
Yes, I agree. However, given how late in the release process I discovered
this, I wanted a fix with absolute minimum impact, which is why I chose the
above. Zero struct change for C code, and enough of a fix for C++ to get it
compiling.
If you feel that replacing with flexible arrays is safe enough to do at
this late stage, we can perhaps consider it, but overall I'd suggest doing
any such replacement in 22.07

/Bruce