Re: kernel header compatibility

[Rich Felker <dalias@libc.org>]

On Wed, Nov 09, 2016 at 08:28:32PM -0500, Felix Janda wrote:
> Rich Felker wrote:
> > On Tue, Nov 08, 2016 at 08:54:23PM -0500, Felix Janda wrote:
> > > Rich Felker wrote:
> > > > On Tue, Nov 08, 2016 at 08:11:45PM -0500, Felix Janda wrote:
> > > > > The recent commit 04983f2272382af92eb8f8838964ff944fbb8258 (make
> > > > > netinet/in.h suppress clashing definitions from kernel headers)
> > > > > intends to address some of the conflicts between the kernel and musl
> > > > > libc headers. Namely it tries to allow the inclusion of kernel headers
> > > > > after libc header by defining __UAP_DEF_* macros. However this doesn't
> > > > > work because the relevant linux headers include <linux/libc-compat.h>,
> > > > > which unconditionally redefines the constants. For example
> > > > 
> > > > Oh, how awful. I missed the whole bogus #else part after the #if
> > > > defined(__GLIBC__) block.
> > > > > 
> > > > > #include <netinet/in.h>
> > > > > #include <linux/in6.h>
> > > > > 
> > > > > leads to
> > > > > 
> > > > > #define __UAPI_DEF_IN_ADDR 0 // from <netinet/in.h>
> > > > > #define __UAPI_DEF_IN_ADDR 1 // from <linux/libc-compat.h>
> > > > 
> > > > Conflicting defines should be an error already.
> > > 
> > > Strangely gcc-6.2.0 just warns (and the warning is not displayed when
> > > it is in system headers).
> > > 
> > > > > So we still get two conflicting definitions of struct in6_addr.
> > > > > 
> > > > > 
> > > > > By adding the hack "#define _LIBC_COMPAT_H" to <netinet/in.h>, this
> > > > > particular example compiles.
> > > > > 
> > > > > 
> > > > > Maybe the kernel people can be convinced to add #ifdef guards around
> > > > > all of the (non glibc) __UAPI_* definitions in <linux/libc-compat.h>.
> > > > 
> > > > I think they should, but I don't mind just suppressing the whole
> > > > header by defining _UAPI_LIBC_COMPAT_H if that works for all kernel
> > > > versions. It seems to; see:
> > > > 
> > > > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/include/uapi/linux/libc-compat.h?id=cfd280c91253cc28e4919e349fa7a813b63e71e8
> > > > 
> > > > where the file was introduced.
> > > 
> > > Note that for the actually installed kernel headers the "_UAPI" prefix
> > > is stripped from the include guard. For example "_UAPI__LINUX_KEYBOARD"
> > > becomes "__LINUX_KEYBOARD".
> > > 
> > > Second, defining the include guard in <netinet/in.h> would prevent
> > > <linux/libc-compat.h> from defining __UAPI_* constants for things
> > > actually missing from musl. For example, a recent <linux/ipx.h> would
> > > no longer define struct sockaddr_ipx when included after
> > > <netinet/in.h>.
> > 
> > Uhg. So there's really no fix except for the kernel to put #ifndef
> > around its definitions of individual macros, is there?
> 
> I just noticed that glibc does not have #if !__UAPI_DEF_FOO guards
> despite the suggestions in <linux/libc-compat.h>. So they also don't
> seem to bother with making inclusion of libc headers after kernel
> headers safe.

No, they do it some other way, looking for the kernel headers'
inclusion guards. See for example sysdeps/unix/sysv/linux/bits/in.h.

> Because of this (no need to coordinate with glibc), it might be
> possible to fix the kernel to use the __UAPI macros in the opposite way
> (have a __UAPI_NODEF_FOO instead of __UAPI_DEF_FOO.) Then it would be
> possible to remove the #if !defined(GLIBC) case in
> <linux/libc-compat.h>.

I would try to avoid changing existing "stable" interfaces; that seems
unnecessarily controversial. Just

#ifndef __UAPI_DEF_FOO
#define __UAPI_DEF_FOO 1
#endif

for each FOO should work okay.

> > Would you be willing to propose such a patch? I'd ack it.
> 
> Yes. (I would sent to linux-devel@vger.kernel.org, and CC David Miller
> and the musl list.)

OK. Thanks!

Rich