On Thu, Aug 29, 2024 at 01:17:53AM +0100, Sam James wrote:
> "Darrick J. Wong" <djwong@xxxxxxxxxx> writes:
>
> > On Wed, Aug 28, 2024 at 01:01:31AM +0100, Sam James wrote:
> >> "Darrick J. Wong" <djwong@xxxxxxxxxx> writes:
> >>
> >> > From: Darrick J. Wong <djwong@xxxxxxxxxx>
> >> >
> >> > Apparently C++ compilers don't like the implicit void* casts that go on
> >> > in the system headers. Compile a dummy program with the C++ compiler to
> >> > make sure this works, so Darrick has /some/ chance of figuring these
> >> > things out before the users do.
> >>
> >> Thanks, this is a good idea. Double thanks for the quick fix.
> >>
> >> 1) yes, it finds the breakage:
> >> Tested-by: Sam James <sam@xxxxxxxxxx>
> >>
> >> 2) with the fix below (CC -> CXX):
> >> Reviewed-by: Sam James <sam@xxxxxxxxxx>
> >>
> >> 3) another thing to think about is:
> >> * -pedantic?
> >
> > -pedantic won't build because C++ doesn't support flexarrays:
> >
> > In file included from ../include/xfs.h:61:
> > ../include/xfs/xfs_fs.h:523:33: error: ISO C++ forbids flexible array member ‘bulkstat’ [-Werror=pedantic]
> > 523 | struct xfs_bulkstat bulkstat[];
> > | ^~~~~~~~
> >
> > even if you wrap it in extern "C" { ... };
>
> Doh. So the ship has kind of sailed already anyway.
>
> >
> >> * maybe do one for a bunch of standards? (I think systemd does every
> >> possible value [1])
> >
> > That might be overkill since xfsprogs' build system doesn't have a good
> > mechanism for detecting if a compiler supports a particular standard.
> > I'm not even sure there's a good "reference" C++ standard to pick here,
> > since the kernel doesn't require a C++ compiler.
> >
> >> * doing the above for C as well
> >
> > Hmm, that's a good idea.
> >
> > I think the only relevant standard here is C11 (well really gnu11),
> > because that's what the kernel compiles with since 5.18. xfsprogs
> > doesn't specify any particular version of C, but perhaps we should match
> > the kernel every time they bump that up?
>
> Projects are often (IMO far too) conservative with the features they use
> in their headers and I don't think it's unreasonable to match the kernel
> here.
>
> >
> > IOWs, should we build xfsprogs with -std=gnu11? The commit changing the
> > kernel to gnu11 (e8c07082a810) remarks that gcc 5.1 supports it just
> > fine. IIRC RHEL 7 only has 4.8.5 but it's now in extended support so
> > ... who cares? The oldest supported Debian stable has gcc 8.
>
> so, I think we should match whatever linux-headers / the uapi rules are,
> and given I've seen flexible array members in there, it's at least C99.
>
> I did some quick greps and am not sure if we're using any C11 features
> in uapi at least.
-std=gnu11 seems like the correct choice, yes.
> Just don't blame me if someone yells ;)
>
> (kees, any idea if I'm talking rubbish?)
>
> tl;dr: let's try gnu11?
>
> > [...]
>
> sam
In really ugly cases (i.e. MSVC importing headers into a C++ project in
ACPICA) we did things like:
#if defined(__cplusplus)
# define __FLEX_SIZE 0
#else
# define __FLEX_SIZE /**/
#endif
...
struct ... {
...
struct xfs_bulkstat bulkstat[__FLEX_SIZE];
...
};
--
Kees Cook
