Re: [PATCH] uapi: fix linux/if.h userspace compilation errors

["Dmitry V. Levin" <ldv@altlinux.org>]

[-- Attachment #1: Type: text/plain, Size: 3245 bytes --]

On Tue, Feb 21, 2017 at 12:10:22PM -0500, David Miller wrote:
> From: "Dmitry V. Levin" <ldv@altlinux.org>
> Date: Mon, 20 Feb 2017 14:58:41 +0300
> 
> > Include <sys/socket.h> (guarded by ifndef __KERNEL__) to fix
> > the following linux/if.h userspace compilation errors:
> 
> Wouldn't it be so much better to do this in include/uapi/linux/socket.h?

Yes, it would be nicer if we could afford it.  However, changing
uapi/linux/socket.h to include <sys/socket.h> is less conservative than
changing every uapi header that fails to compile because of its use
of struct sockaddr.  It's risky because <sys/socket.h> pulls in other
types that might conflict with definitions provided by uapi headers.

I've checked all uapi headers that have no compilation errors whether
inclusion of <sys/socket.h> before them causes any issues, and found
that linux/time.h and linux/uio.h (and their users) stop compiling
because of type conflicts:

$ gcc -S -o/dev/null -xc /dev/null -include /usr/include/sys/socket.h -include /usr/include/linux/time.h
In file included from <command-line>:32:0:
/usr/include/linux/time.h:9:8: error: redefinition of 'struct timespec'
 struct timespec {
        ^~~~~~~~
In file included from /usr/include/sys/select.h:45:0,
                 from /usr/include/sys/types.h:219,
                 from /usr/include/sys/uio.h:23,
                 from /usr/include/sys/socket.h:26,
                 from <command-line>:32:
/usr/include/time.h:120:8: note: originally defined here
 struct timespec
        ^~~~~~~~
In file included from <command-line>:32:0:
/usr/include/linux/time.h:15:8: error: redefinition of 'struct timeval'
 struct timeval {
        ^~~~~~~
In file included from /usr/include/sys/select.h:47:0,
                 from /usr/include/sys/types.h:219,
                 from /usr/include/sys/uio.h:23,
                 from /usr/include/sys/socket.h:26,
                 from <command-line>:32:
/usr/include/bits/time.h:30:8: note: originally defined here
 struct timeval
        ^~~~~~~
$ gcc -S -o/dev/null -xc /dev/null -include /usr/include/sys/socket.h -include /usr/include/linux/uio.h
In file included from <command-line>:32:0:
/usr/include/linux/uio.h:16:8: error: redefinition of 'struct iovec'
 struct iovec
        ^~~~~
In file included from /usr/include/sys/uio.h:28:0,
                 from /usr/include/sys/socket.h:26,
                 from <command-line>:32:
/usr/include/bits/uio.h:43:8: note: originally defined here
 struct iovec
        ^~~~~

We can try to workaround these ancient <time.h> against <linux/time.h>
and <sys/uio.h> against <linux/uio.h> conflicts on uapi side by
introducing __UAPI_DEF_TIMESPEC, __UAPI_DEF_TIMEVAL, and __UAPI_DEF_IOVEC
guards, but these workarounds will work only one way (when libc headers
are included before uapi ones) until glibc catches up, and the latter
may take a lot of time.

> I mean, look at the comment right by the change you are making:
> 
> > @@ -24,6 +24,10 @@
> >  #include <linux/socket.h>		/* for "struct sockaddr" et al	*/
> 
> Anyone including linux/socket.h is trying to obtain that type.

I think this comment is true for #ifdef __KERNEL__ code only.


-- 
ldv

[-- Attachment #2: Type: application/pgp-signature, Size: 801 bytes --]