Re: [RFC PATCH] libxfs: compile with a C++ compiler

[Kees Cook <kees@kernel.org>]

On Thu, Aug 29, 2024 at 01:17:53AM +0100, Sam James wrote:
> "Darrick J. Wong" <djwong@kernel.org> writes:
> 
> > On Wed, Aug 28, 2024 at 01:01:31AM +0100, Sam James wrote:
> >> "Darrick J. Wong" <djwong@kernel.org> writes:
> >> 
> >> > From: Darrick J. Wong <djwong@kernel.org>
> >> >
> >> > 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@gentoo.org>
> >> 
> >> 2) with the fix below (CC -> CXX):
> >> Reviewed-by: Sam James <sam@gentoo.org>
> >> 
> >> 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