2017-07-08 20:29:23

by Felix Janda

[permalink] [raw]
Subject: [PATCHv2] uapi libc compat: add fallback for unsupported libcs

libc-compat.h aims to prevent symbol collisions between uapi and libc
headers for each supported libc. This requires continuous coordination
between them.

The goal of this commit is to improve the situation for libcs (such as
musl) which are not yet supported and/or do not wish to be explicitly
supported, while not affecting supported libcs. More precisely, with
this commit, unsupported libcs can request the suppression of any
specific uapi definition by defining the correspondings _UAPI_DEF_*
macro as 0. This can fix symbol collisions for them, as long as the
libc headers are included before the uapi headers. Inclusion in the
other order is outside the scope of this commit.

All infrastructure in order to enable this fallback for unsupported
libcs is already in place, except that libc-compat.h unconditionally
defines all _UAPI_DEF_* macros to 1 for all unsupported libcs so that
any previous definitions are ignored. In order to fix this, this commit
merely makes these definitions conditional.

This commit together with the musl libc commit

http://git.musl-libc.org/cgit/musl/commit/?id=04983f2272382af92eb8f8838964ff944fbb8258

fixes for example the following compiler errors when <linux/in6.h> is
included after musl's <netinet/in.h>:

./linux/in6.h:32:8: error: redefinition of 'struct in6_addr'
./linux/in6.h:49:8: error: redefinition of 'struct sockaddr_in6'
./linux/in6.h:59:8: error: redefinition of 'struct ipv6_mreq'

Signed-off-by: Felix Janda <[email protected]>
Acked-by: Rich Felker <[email protected]>
---
v2: The only change to the previous version is the commit title and
message.
---
include/uapi/linux/libc-compat.h | 52 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 52 insertions(+)

diff --git a/include/uapi/linux/libc-compat.h b/include/uapi/linux/libc-compat.h
index 44b8a6b..c316725 100644
--- a/include/uapi/linux/libc-compat.h
+++ b/include/uapi/linux/libc-compat.h
@@ -171,42 +171,94 @@
#else /* !defined(__GLIBC__) */

/* Definitions for if.h */
+#if !defined(__UAPI_DEF_IF_IFCONF)
#define __UAPI_DEF_IF_IFCONF 1
+#endif
+#if !defined(__UAPI_DEF_IF_IFMAP)
#define __UAPI_DEF_IF_IFMAP 1
+#endif
+#if !defined(__UAPI_DEF_IFNAMSIZ)
#define __UAPI_DEF_IF_IFNAMSIZ 1
+#endif
+#if !defined(__UAPI_DEF_IFREQ)
#define __UAPI_DEF_IF_IFREQ 1
+#endif
/* Everything up to IFF_DYNAMIC, matches net/if.h until glibc 2.23 */
+#if !defined(__UAPI_DEF_IF_NET_DEVICE_FLAGS)
#define __UAPI_DEF_IF_NET_DEVICE_FLAGS 1
+#endif
/* For the future if glibc adds IFF_LOWER_UP, IFF_DORMANT and IFF_ECHO */
+#if !defined(__UAPI_DEF_IF_NET_DEVICE_FLAGS_LOWER_UP_DORMANT_ECHO)
#define __UAPI_DEF_IF_NET_DEVICE_FLAGS_LOWER_UP_DORMANT_ECHO 1
+#endif

/* Definitions for in.h */
+#if !defined(__UAPI_DEF_IN_ADDR)
#define __UAPI_DEF_IN_ADDR 1
+#endif
+#if !defined(__UAPI_DEF_IN_IPPROTO)
#define __UAPI_DEF_IN_IPPROTO 1
+#endif
+#if !defined(__UAPI_DEF_IN_PKTINFO)
#define __UAPI_DEF_IN_PKTINFO 1
+#endif
+#if !defined(__UAPI_DEF_IP_MREQ)
#define __UAPI_DEF_IP_MREQ 1
+#endif
+#if !defined(__UAPI_DEF_SOCKADDR_IN)
#define __UAPI_DEF_SOCKADDR_IN 1
+#endif
+#if !defined(__UAPI_DEF_IN_CLASS)
#define __UAPI_DEF_IN_CLASS 1
+#endif

/* Definitions for in6.h */
+#if !defined(__UAPI_DEF_IN6_ADDR)
#define __UAPI_DEF_IN6_ADDR 1
+#endif
+#if !defined(__UAPI_DEF_IN6_ADDR_ALT)
#define __UAPI_DEF_IN6_ADDR_ALT 1
+#endif
+#if !defined(__UAPI_DEF_SOCKADDR_IN6)
#define __UAPI_DEF_SOCKADDR_IN6 1
+#endif
+#if !defined(__UAPI_DEF_IPV6_MREQ)
#define __UAPI_DEF_IPV6_MREQ 1
+#endif
+#if !defined(__UAPI_DEF_IPPROTO_V6)
#define __UAPI_DEF_IPPROTO_V6 1
+#endif
+#if !defined(__UAPI_DEF_IPV6_OPTIONS)
#define __UAPI_DEF_IPV6_OPTIONS 1
+#endif
+#if !defined(__UAPI_DEF_IN6_PKTINFO)
#define __UAPI_DEF_IN6_PKTINFO 1
+#endif
+#if !defined(__UAPI_DEF_IP6_MTUINFO)
#define __UAPI_DEF_IP6_MTUINFO 1
+#endif

/* Definitions for ipx.h */
+#if !defined(__UAPI_DEF_SOCKADDR_IPX)
#define __UAPI_DEF_SOCKADDR_IPX 1
+#endif
+#if !defined(__UAPI_DEF_IPX_ROUTE_DEFINITION)
#define __UAPI_DEF_IPX_ROUTE_DEFINITION 1
+#endif
+#if !defined(__UAPI_DEF_IPX_INTERFACE_DEFINITION)
#define __UAPI_DEF_IPX_INTERFACE_DEFINITION 1
+#endif
+#if !defined(__UAPI_DEF_IPX_CONFIG_DATA)
#define __UAPI_DEF_IPX_CONFIG_DATA 1
+#endif
+#if !defined(__UAPI_DEF_IPX_ROUTE_DEF)
#define __UAPI_DEF_IPX_ROUTE_DEF 1
+#endif

/* Definitions for xattr.h */
+#if !defined(__UAPI_DEF_XATTR)
#define __UAPI_DEF_XATTR 1
+#endif

#endif /* __GLIBC__ */

--
2.7.3


2017-07-08 21:14:38

by Hauke Mehrtens

[permalink] [raw]
Subject: Re: [musl] [PATCHv2] uapi libc compat: add fallback for unsupported libcs



On 07/08/2017 10:27 PM, Felix Janda wrote:
> libc-compat.h aims to prevent symbol collisions between uapi and libc
> headers for each supported libc. This requires continuous coordination
> between them.
>
> The goal of this commit is to improve the situation for libcs (such as
> musl) which are not yet supported and/or do not wish to be explicitly
> supported, while not affecting supported libcs. More precisely, with
> this commit, unsupported libcs can request the suppression of any
> specific uapi definition by defining the correspondings _UAPI_DEF_*
> macro as 0. This can fix symbol collisions for them, as long as the
> libc headers are included before the uapi headers. Inclusion in the
> other order is outside the scope of this commit.
>
> All infrastructure in order to enable this fallback for unsupported
> libcs is already in place, except that libc-compat.h unconditionally
> defines all _UAPI_DEF_* macros to 1 for all unsupported libcs so that
> any previous definitions are ignored. In order to fix this, this commit
> merely makes these definitions conditional.
>
> This commit together with the musl libc commit
>
> http://git.musl-libc.org/cgit/musl/commit/?id=04983f2272382af92eb8f8838964ff944fbb8258
>
> fixes for example the following compiler errors when <linux/in6.h> is
> included after musl's <netinet/in.h>:
>
> ./linux/in6.h:32:8: error: redefinition of 'struct in6_addr'
> ./linux/in6.h:49:8: error: redefinition of 'struct sockaddr_in6'
> ./linux/in6.h:59:8: error: redefinition of 'struct ipv6_mreq'
>
> Signed-off-by: Felix Janda <[email protected]>
> Acked-by: Rich Felker <[email protected]>
> ---
> v2: The only change to the previous version is the commit title and
> message.
> ---
> include/uapi/linux/libc-compat.h | 52 ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 52 insertions(+)

Does the documentation at the top of this file need some updates?

> diff --git a/include/uapi/linux/libc-compat.h b/include/uapi/linux/libc-compat.h
> index 44b8a6b..c316725 100644
> --- a/include/uapi/linux/libc-compat.h
> +++ b/include/uapi/linux/libc-compat.h
> @@ -171,42 +171,94 @@
> #else /* !defined(__GLIBC__) */

Why don't you used #ifndef ?

> /* Definitions for if.h */
> +#if !defined(__UAPI_DEF_IF_IFCONF)
> #define __UAPI_DEF_IF_IFCONF 1
> +#endif
> +#if !defined(__UAPI_DEF_IF_IFMAP)
> #define __UAPI_DEF_IF_IFMAP 1
> +#endif
> +#if !defined(__UAPI_DEF_IFNAMSIZ)

This should be:
#if !defined( __UAPI_DEF_IF_IFNAMSIZ)

> #define __UAPI_DEF_IF_IFNAMSIZ 1
> +#endif
> +#if !defined(__UAPI_DEF_IFREQ)

This should be:
#if !defined(__UAPI_DEF_IF_IFREQ)

> #define __UAPI_DEF_IF_IFREQ 1
> +#endif
> /* Everything up to IFF_DYNAMIC, matches net/if.h until glibc 2.23 */
> +#if !defined(__UAPI_DEF_IF_NET_DEVICE_FLAGS)
> #define __UAPI_DEF_IF_NET_DEVICE_FLAGS 1
> +#endif
> /* For the future if glibc adds IFF_LOWER_UP, IFF_DORMANT and IFF_ECHO */
> +#if !defined(__UAPI_DEF_IF_NET_DEVICE_FLAGS_LOWER_UP_DORMANT_ECHO)
> #define __UAPI_DEF_IF_NET_DEVICE_FLAGS_LOWER_UP_DORMANT_ECHO 1
> +#endif
>
> /* Definitions for in.h */
> +#if !defined(__UAPI_DEF_IN_ADDR)
> #define __UAPI_DEF_IN_ADDR 1
> +#endif
> +#if !defined(__UAPI_DEF_IN_IPPROTO)
> #define __UAPI_DEF_IN_IPPROTO 1
> +#endif
> +#if !defined(__UAPI_DEF_IN_PKTINFO)
> #define __UAPI_DEF_IN_PKTINFO 1
> +#endif
> +#if !defined(__UAPI_DEF_IP_MREQ)
> #define __UAPI_DEF_IP_MREQ 1
> +#endif
> +#if !defined(__UAPI_DEF_SOCKADDR_IN)
> #define __UAPI_DEF_SOCKADDR_IN 1
> +#endif
> +#if !defined(__UAPI_DEF_IN_CLASS)
> #define __UAPI_DEF_IN_CLASS 1
> +#endif
>
> /* Definitions for in6.h */
> +#if !defined(__UAPI_DEF_IN6_ADDR)
> #define __UAPI_DEF_IN6_ADDR 1
> +#endif
> +#if !defined(__UAPI_DEF_IN6_ADDR_ALT)
> #define __UAPI_DEF_IN6_ADDR_ALT 1
> +#endif
> +#if !defined(__UAPI_DEF_SOCKADDR_IN6)
> #define __UAPI_DEF_SOCKADDR_IN6 1
> +#endif
> +#if !defined(__UAPI_DEF_IPV6_MREQ)
> #define __UAPI_DEF_IPV6_MREQ 1
> +#endif
> +#if !defined(__UAPI_DEF_IPPROTO_V6)
> #define __UAPI_DEF_IPPROTO_V6 1
> +#endif
> +#if !defined(__UAPI_DEF_IPV6_OPTIONS)
> #define __UAPI_DEF_IPV6_OPTIONS 1
> +#endif
> +#if !defined(__UAPI_DEF_IN6_PKTINFO)
> #define __UAPI_DEF_IN6_PKTINFO 1
> +#endif
> +#if !defined(__UAPI_DEF_IP6_MTUINFO)
> #define __UAPI_DEF_IP6_MTUINFO 1
> +#endif
>
> /* Definitions for ipx.h */
> +#if !defined(__UAPI_DEF_SOCKADDR_IPX)
> #define __UAPI_DEF_SOCKADDR_IPX 1
> +#endif
> +#if !defined(__UAPI_DEF_IPX_ROUTE_DEFINITION)
> #define __UAPI_DEF_IPX_ROUTE_DEFINITION 1
> +#endif
> +#if !defined(__UAPI_DEF_IPX_INTERFACE_DEFINITION)
> #define __UAPI_DEF_IPX_INTERFACE_DEFINITION 1
> +#endif
> +#if !defined(__UAPI_DEF_IPX_CONFIG_DATA)
> #define __UAPI_DEF_IPX_CONFIG_DATA 1
> +#endif
> +#if !defined(__UAPI_DEF_IPX_ROUTE_DEF)
> #define __UAPI_DEF_IPX_ROUTE_DEF 1
> +#endif
>
> /* Definitions for xattr.h */
> +#if !defined(__UAPI_DEF_XATTR)
> #define __UAPI_DEF_XATTR 1
> +#endif
>
> #endif /* __GLIBC__ */
>
>

2017-07-08 21:45:52

by Felix Janda

[permalink] [raw]
Subject: Re: [musl] [PATCHv2] uapi libc compat: add fallback for unsupported libcs

Hauke Mehrtens wrote:
>
>
> On 07/08/2017 10:27 PM, Felix Janda wrote:
> > libc-compat.h aims to prevent symbol collisions between uapi and libc
> > headers for each supported libc. This requires continuous coordination
> > between them.
> >
> > The goal of this commit is to improve the situation for libcs (such as
> > musl) which are not yet supported and/or do not wish to be explicitly
> > supported, while not affecting supported libcs. More precisely, with
> > this commit, unsupported libcs can request the suppression of any
> > specific uapi definition by defining the correspondings _UAPI_DEF_*
> > macro as 0. This can fix symbol collisions for them, as long as the
> > libc headers are included before the uapi headers. Inclusion in the
> > other order is outside the scope of this commit.
> >
> > All infrastructure in order to enable this fallback for unsupported
> > libcs is already in place, except that libc-compat.h unconditionally
> > defines all _UAPI_DEF_* macros to 1 for all unsupported libcs so that
> > any previous definitions are ignored. In order to fix this, this commit
> > merely makes these definitions conditional.
> >
> > This commit together with the musl libc commit
> >
> > http://git.musl-libc.org/cgit/musl/commit/?id=04983f2272382af92eb8f8838964ff944fbb8258
> >
> > fixes for example the following compiler errors when <linux/in6.h> is
> > included after musl's <netinet/in.h>:
> >
> > ./linux/in6.h:32:8: error: redefinition of 'struct in6_addr'
> > ./linux/in6.h:49:8: error: redefinition of 'struct sockaddr_in6'
> > ./linux/in6.h:59:8: error: redefinition of 'struct ipv6_mreq'
> >
> > Signed-off-by: Felix Janda <[email protected]>
> > Acked-by: Rich Felker <[email protected]>
> > ---
> > v2: The only change to the previous version is the commit title and
> > message.
> > ---
> > include/uapi/linux/libc-compat.h | 52 ++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 52 insertions(+)
>
> Does the documentation at the top of this file need some updates?

I think that at least the comment preceding the part touched by this
patch should be updated. I'm not sure how to update the documentation
on top -- it does not say anything about unsupported libcs.

> > diff --git a/include/uapi/linux/libc-compat.h b/include/uapi/linux/libc-compat.h
> > index 44b8a6b..c316725 100644
> > --- a/include/uapi/linux/libc-compat.h
> > +++ b/include/uapi/linux/libc-compat.h
> > @@ -171,42 +171,94 @@
> > #else /* !defined(__GLIBC__) */
>
> Why don't you used #ifndef ?

It seemed more consistent with the rest of the file, which also uses
"#if defined(...)" instead of "#ifdef". On the other hand, there are
two uses of "#ifndef" in the file. I do not have any strong opinion on
this.

> > /* Definitions for if.h */
> > +#if !defined(__UAPI_DEF_IF_IFCONF)
> > #define __UAPI_DEF_IF_IFCONF 1
> > +#endif
> > +#if !defined(__UAPI_DEF_IF_IFMAP)
> > #define __UAPI_DEF_IF_IFMAP 1
> > +#endif
> > +#if !defined(__UAPI_DEF_IFNAMSIZ)
>
> This should be:
> #if !defined( __UAPI_DEF_IF_IFNAMSIZ)
>
> > #define __UAPI_DEF_IF_IFNAMSIZ 1
> > +#endif
> > +#if !defined(__UAPI_DEF_IFREQ)
>
> This should be:
> #if !defined(__UAPI_DEF_IF_IFREQ)

Thanks! Will be fixed in the next version.

Felix

> > #define __UAPI_DEF_IF_IFREQ 1
> > +#endif
> > /* Everything up to IFF_DYNAMIC, matches net/if.h until glibc 2.23 */
> > +#if !defined(__UAPI_DEF_IF_NET_DEVICE_FLAGS)
> > #define __UAPI_DEF_IF_NET_DEVICE_FLAGS 1
> > +#endif
> > /* For the future if glibc adds IFF_LOWER_UP, IFF_DORMANT and IFF_ECHO */
> > +#if !defined(__UAPI_DEF_IF_NET_DEVICE_FLAGS_LOWER_UP_DORMANT_ECHO)
> > #define __UAPI_DEF_IF_NET_DEVICE_FLAGS_LOWER_UP_DORMANT_ECHO 1
> > +#endif
> >
> > /* Definitions for in.h */
> > +#if !defined(__UAPI_DEF_IN_ADDR)
> > #define __UAPI_DEF_IN_ADDR 1
> > +#endif
> > +#if !defined(__UAPI_DEF_IN_IPPROTO)
> > #define __UAPI_DEF_IN_IPPROTO 1
> > +#endif
> > +#if !defined(__UAPI_DEF_IN_PKTINFO)
> > #define __UAPI_DEF_IN_PKTINFO 1
> > +#endif
> > +#if !defined(__UAPI_DEF_IP_MREQ)
> > #define __UAPI_DEF_IP_MREQ 1
> > +#endif
> > +#if !defined(__UAPI_DEF_SOCKADDR_IN)
> > #define __UAPI_DEF_SOCKADDR_IN 1
> > +#endif
> > +#if !defined(__UAPI_DEF_IN_CLASS)
> > #define __UAPI_DEF_IN_CLASS 1
> > +#endif
> >
> > /* Definitions for in6.h */
> > +#if !defined(__UAPI_DEF_IN6_ADDR)
> > #define __UAPI_DEF_IN6_ADDR 1
> > +#endif
> > +#if !defined(__UAPI_DEF_IN6_ADDR_ALT)
> > #define __UAPI_DEF_IN6_ADDR_ALT 1
> > +#endif
> > +#if !defined(__UAPI_DEF_SOCKADDR_IN6)
> > #define __UAPI_DEF_SOCKADDR_IN6 1
> > +#endif
> > +#if !defined(__UAPI_DEF_IPV6_MREQ)
> > #define __UAPI_DEF_IPV6_MREQ 1
> > +#endif
> > +#if !defined(__UAPI_DEF_IPPROTO_V6)
> > #define __UAPI_DEF_IPPROTO_V6 1
> > +#endif
> > +#if !defined(__UAPI_DEF_IPV6_OPTIONS)
> > #define __UAPI_DEF_IPV6_OPTIONS 1
> > +#endif
> > +#if !defined(__UAPI_DEF_IN6_PKTINFO)
> > #define __UAPI_DEF_IN6_PKTINFO 1
> > +#endif
> > +#if !defined(__UAPI_DEF_IP6_MTUINFO)
> > #define __UAPI_DEF_IP6_MTUINFO 1
> > +#endif
> >
> > /* Definitions for ipx.h */
> > +#if !defined(__UAPI_DEF_SOCKADDR_IPX)
> > #define __UAPI_DEF_SOCKADDR_IPX 1
> > +#endif
> > +#if !defined(__UAPI_DEF_IPX_ROUTE_DEFINITION)
> > #define __UAPI_DEF_IPX_ROUTE_DEFINITION 1
> > +#endif
> > +#if !defined(__UAPI_DEF_IPX_INTERFACE_DEFINITION)
> > #define __UAPI_DEF_IPX_INTERFACE_DEFINITION 1
> > +#endif
> > +#if !defined(__UAPI_DEF_IPX_CONFIG_DATA)
> > #define __UAPI_DEF_IPX_CONFIG_DATA 1
> > +#endif
> > +#if !defined(__UAPI_DEF_IPX_ROUTE_DEF)
> > #define __UAPI_DEF_IPX_ROUTE_DEF 1
> > +#endif
> >
> > /* Definitions for xattr.h */
> > +#if !defined(__UAPI_DEF_XATTR)
> > #define __UAPI_DEF_XATTR 1
> > +#endif
> >
> > #endif /* __GLIBC__ */
> >
> >

--