[5/5] eventdev: fix compilation with clang C++ builds
Checks
Commit Message
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(-)
Comments
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:
// SPDX-License-Identifier: GPL-2.0-only
///
/// Zero-length and one-element arrays are deprecated, see
/// Documentation/process/deprecated.rst
/// Flexible-array members should be used instead.
///
//
// Confidence: High
// Copyright: (C) 2020 Denis Efremov ISPRAS.
// Comments:
// Options: --no-includes --include-headers
virtual context
virtual report
virtual org
virtual patch
@initialize:python@
@@
def relevant(positions):
for p in positions:
if "uapi" in p.file:
return False
return True
@r depends on !patch@
identifier name, array;
type T;
position p : script:python() { relevant(p) };
@@
(
struct name {
...
* T array@p[\(0\|1\)];
};
|
struct {
...
* T array@p[\(0\|1\)];
};
|
union name {
...
* T array@p[\(0\|1\)];
};
|
union {
...
* T array@p[\(0\|1\)];
};
)
@only_field depends on patch@
identifier name, array;
type T;
position q;
@@
(
struct name {@q
T array[0];
};
|
struct {@q
T array[0];
};
)
@depends on patch@
identifier name, array;
type T;
position p : script:python() { relevant(p) };
// position @q with rule "only_field" simplifies
// handling of bitfields, arrays, etc.
position q != only_field.q;
@@
(
struct name {@q
...
T array@p[
- 0
];
};
|
struct {@q
...
T array@p[
- 0
];
};
)
@script: python depends on report@
p << r.p;
@@
msg = "WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)"
coccilib.report.print_report(p[0], msg)
@script: python depends on org@
p << r.p;
@@
msg = "WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays)"
coccilib.org.print_todo(p[0], msg)
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
@@ -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