linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next 0/2] uapi: include time.h from errqueue.h
@ 2016-09-12 17:05 Willem de Bruijn
  2016-09-12 17:05 ` [PATCH net-next 1/2] uapi glibc compat: make linux/time.h compile with user time.h files Willem de Bruijn
  2016-09-12 17:05 ` [PATCH net-next 2/2] errqueue: include linux/time.h Willem de Bruijn
  0 siblings, 2 replies; 5+ messages in thread
From: Willem de Bruijn @ 2016-09-12 17:05 UTC (permalink / raw)
  To: netdev; +Cc: davem, linux-kernel, john.stultz, bmoses, Willem de Bruijn

From: Willem de Bruijn <willemb@google.com>

It was reported that linux/errqueue.h requires linux/time.h, but that
adding the include directly may cause userspace conflicts between
linux/time.h and glibc time.h:

  https://lkml.org/lkml/2016/7/10/10

Address the conflicts using the standard libc-compat approach, then
add the #include to errqueue.h

The first patch is a resubmit. It was previously submitted to
tip/timers/core, but given the commit history, the maintainer
suggested this tree, instead.

  https://lkml.org/lkml/2016/8/10/748

This also allows sending the follow-up as part of the patchset.

Willem de Bruijn (2):
  uapi glibc compat: make linux/time.h compile with user time.h files
  errqueue: include linux/time.h

 include/uapi/linux/errqueue.h    |  1 +
 include/uapi/linux/libc-compat.h | 50 ++++++++++++++++++++++++++++++++++++++++
 include/uapi/linux/time.h        | 15 ++++++++++++
 3 files changed, 66 insertions(+)

-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply	[flat|nested] 5+ messages in thread
* [PATCH net-next 1/2] uapi glibc compat: make linux/time.h compile with user time.h files
  2016-09-12 17:05 [PATCH net-next 0/2] uapi: include time.h from errqueue.h Willem de Bruijn
@ 2016-09-12 17:05 ` Willem de Bruijn
  2016-09-12 17:05 ` [PATCH net-next 2/2] errqueue: include linux/time.h Willem de Bruijn
  1 sibling, 0 replies; 5+ messages in thread
From: Willem de Bruijn @ 2016-09-12 17:05 UTC (permalink / raw)
  To: netdev; +Cc: davem, linux-kernel, john.stultz, bmoses, Willem de Bruijn

From: Willem de Bruijn <willemb@google.com>

Add libc-compat workaround for definitions in linux/time.h that
duplicate those in libc time.h, sys/time.h and bits/time.h.

With this change, userspace builds succeeds when linux/time.h is
included after libc time.h and when it is included after sys/time.h.

The inverse requires additional changes to those userspace headers.

Without this patch, when compiling the following program after
make headers_install:

  echo -e "#include <time.h>\n#include <linux/time.h>" | \
  	gcc -Wall -Werror -Iusr/include -c -xc -

gcc gives these errors:

  #include <time.h>
  #include <linux/time.h>

    In file included from ../test_time.c:3:0:
    /usr/include/time.h:120:8: error: redefinition of ‘struct timespec’
     struct timespec
    	^
    In file included from ../test_time.c:2:0:
    ./usr/include/linux/time.h:9:8: note: originally defined here
     struct timespec {
    	^
    In file included from ../test_time.c:3:0:
    /usr/include/time.h:161:8: error: redefinition of ‘struct itimerspec’
     struct itimerspec
    	^
    In file included from ../test_time.c:2:0:
    ./usr/include/linux/time.h:34:8: note: originally defined here
     struct itimerspec {

and this warning by indirect inclusion of bits/time.h:

    In file included from ../test_time.c:4:0:
    ./usr/include/linux/time.h:67:0: error: "TIMER_ABSTIME" redefined [-Werror]
     #define TIMER_ABSTIME   0x01
     ^
    In file included from /usr/include/time.h:41:0,
    		 from ../test_time.c:3:
    /usr/include/x86_64-linux-gnu/bits/time.h:82:0: note: this is the location of the previous definition
     #   define TIMER_ABSTIME  1
     ^

The _SYS_TIME_H variant resolves similar errors for timeval, timezone,
itimerval and warnings for ITIMER_REAL, ITIMER_VIRTUAL, ITIMER_PROF.

Ran the same program for sys/time.h and bits/time.h.

Signed-off-by: Willem de Bruijn <willemb@google.com>
---
 include/uapi/linux/libc-compat.h | 50 ++++++++++++++++++++++++++++++++++++++++
 include/uapi/linux/time.h        | 15 ++++++++++++
 2 files changed, 65 insertions(+)

diff --git a/include/uapi/linux/libc-compat.h b/include/uapi/linux/libc-compat.h
index 44b8a6b..b08b0f5 100644
--- a/include/uapi/linux/libc-compat.h
+++ b/include/uapi/linux/libc-compat.h
@@ -165,6 +165,43 @@
 #define __UAPI_DEF_XATTR		1
 #endif
 
+/* Definitions for time.h */
+#if defined(__timespec_defined)
+#define __UAPI_DEF_TIMESPEC		0
+#else
+#define __UAPI_DEF_TIMESPEC		1
+#endif
+
+#if defined(_TIME_H) && defined(__USE_POSIX199309)
+#define __UAPI_DEF_ITIMERSPEC		0
+#else
+#define __UAPI_DEF_ITIMERSPEC		1
+#endif
+
+/* Definitions for sys/time.h */
+#if defined(_SYS_TIME_H)
+#define __UAPI_DEF_TIMEVAL		0
+#define __UAPI_DEF_ITIMERVAL		0
+#define __UAPI_DEF_ITIMER_WHICH		0
+#else
+#define __UAPI_DEF_TIMEVAL		1
+#define __UAPI_DEF_ITIMERVAL		1
+#define __UAPI_DEF_ITIMER_WHICH		1
+#endif
+
+/* Definitions for bits/time.h */
+#if defined(_BITS_TIME_H)
+#define __UAPI_DEF_ABSTIME		0
+#else
+#define __UAPI_DEF_ABSTIME		1
+#endif
+
+#if defined(_SYS_TIME_H) && defined(__USE_BSD)
+#define __UAPI_DEF_TIMEZONE		0
+#else
+#define __UAPI_DEF_TIMEZONE		1
+#endif
+
 /* If we did not see any headers from any supported C libraries,
  * or we are being included in the kernel, then define everything
  * that we need. */
@@ -208,6 +245,19 @@
 /* Definitions for xattr.h */
 #define __UAPI_DEF_XATTR		1
 
+/* Definitions for time.h */
+#define __UAPI_DEF_TIMESPEC		1
+#define __UAPI_DEF_ITIMERSPEC		1
+
+/* Definitions for sys/time.h */
+#define __UAPI_DEF_TIMEVAL		1
+#define __UAPI_DEF_ITIMERVAL		1
+#define __UAPI_DEF_ITIMER_WHICH		1
+#define __UAPI_DEF_TIMEZONE		1
+
+/* Definitions for bits/time.h */
+#define __UAPI_DEF_ABSTIME		1
+
 #endif /* __GLIBC__ */
 
 #endif /* _UAPI_LIBC_COMPAT_H */
diff --git a/include/uapi/linux/time.h b/include/uapi/linux/time.h
index e75e1b6..4e7333c 100644
--- a/include/uapi/linux/time.h
+++ b/include/uapi/linux/time.h
@@ -1,9 +1,11 @@
 #ifndef _UAPI_LINUX_TIME_H
 #define _UAPI_LINUX_TIME_H
 
+#include <linux/libc-compat.h>
 #include <linux/types.h>
 
 
+#if __UAPI_DEF_TIMESPEC
 #ifndef _STRUCT_TIMESPEC
 #define _STRUCT_TIMESPEC
 struct timespec {
@@ -11,35 +13,46 @@ struct timespec {
 	long		tv_nsec;		/* nanoseconds */
 };
 #endif
+#endif
 
+#if __UAPI_DEF_TIMEVAL
 struct timeval {
 	__kernel_time_t		tv_sec;		/* seconds */
 	__kernel_suseconds_t	tv_usec;	/* microseconds */
 };
+#endif
 
+#if __UAPI_DEF_TIMEZONE
 struct timezone {
 	int	tz_minuteswest;	/* minutes west of Greenwich */
 	int	tz_dsttime;	/* type of dst correction */
 };
+#endif
 
 
 /*
  * Names of the interval timers, and structure
  * defining a timer setting:
  */
+#if __UAPI_DEF_ITIMER_WHICH
 #define	ITIMER_REAL		0
 #define	ITIMER_VIRTUAL		1
 #define	ITIMER_PROF		2
+#endif
 
+#if __UAPI_DEF_ITIMERSPEC
 struct itimerspec {
 	struct timespec it_interval;	/* timer period */
 	struct timespec it_value;	/* timer expiration */
 };
+#endif
 
+#if __UAPI_DEF_ITIMERVAL
 struct itimerval {
 	struct timeval it_interval;	/* timer interval */
 	struct timeval it_value;	/* current value */
 };
+#endif
 
 /*
  * The IDs of the various system clocks (for POSIX.1b interval timers):
@@ -64,6 +77,8 @@ struct itimerval {
 /*
  * The various flags for setting POSIX.1b interval timers:
  */
+#if __UAPI_DEF_ABSTIME
 #define TIMER_ABSTIME			0x01
+#endif
 
 #endif /* _UAPI_LINUX_TIME_H */
-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* [PATCH net-next 2/2] errqueue: include linux/time.h
  2016-09-12 17:05 [PATCH net-next 0/2] uapi: include time.h from errqueue.h Willem de Bruijn
  2016-09-12 17:05 ` [PATCH net-next 1/2] uapi glibc compat: make linux/time.h compile with user time.h files Willem de Bruijn
@ 2016-09-12 17:05 ` Willem de Bruijn
  2016-09-12 19:26   ` kbuild test robot
  1 sibling, 1 reply; 5+ messages in thread
From: Willem de Bruijn @ 2016-09-12 17:05 UTC (permalink / raw)
  To: netdev; +Cc: davem, linux-kernel, john.stultz, bmoses, Willem de Bruijn

From: Willem de Bruijn <willemb@google.com>

struct scm_timestamping has fields of type struct timespec. Now that
it is safe to include linux/time.h and time.h at the same time,
include linux/time.h directly in linux/errqueue.h

Without this patch, when compiling the following program after
make headers_install:

gcc -Wall -Werror -Iusr/include -c -xc - <<EOF
  #include <linux/errqueue.h>
  static struct scm_timestamping tss;
  int main(void) { tss.ts[0].tv_sec = 1; return 0; }
EOF

gcc gives this error:

  In file included from <stdin>:1:0:
  usr/include/linux/errqueue.h:33:18: error: array type has incomplete element type
    struct timespec ts[3];

Reported-by: Brooks Moses <bmoses@google.com>
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
 include/uapi/linux/errqueue.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/uapi/linux/errqueue.h b/include/uapi/linux/errqueue.h
index 07bdce1..abafec8 100644
--- a/include/uapi/linux/errqueue.h
+++ b/include/uapi/linux/errqueue.h
@@ -1,6 +1,7 @@
 #ifndef _UAPI_LINUX_ERRQUEUE_H
 #define _UAPI_LINUX_ERRQUEUE_H
 
+#include <linux/time.h>
 #include <linux/types.h>
 
 struct sock_extended_err {
-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH net-next 2/2] errqueue: include linux/time.h
  2016-09-12 17:05 ` [PATCH net-next 2/2] errqueue: include linux/time.h Willem de Bruijn
@ 2016-09-12 19:26   ` kbuild test robot
  2016-09-12 20:09     ` Willem de Bruijn
  0 siblings, 1 reply; 5+ messages in thread
From: kbuild test robot @ 2016-09-12 19:26 UTC (permalink / raw)
  To: Willem de Bruijn
  Cc: kbuild-all, netdev, davem, linux-kernel, john.stultz, bmoses,
	Willem de Bruijn

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

Hi Willem,

[auto build test ERROR on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Willem-de-Bruijn/uapi-include-time-h-from-errqueue-h/20160913-020431
config: i386-defconfig (attached as .config)
compiler: gcc-6 (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   In file included from ./usr/include/linux/errqueue.h:4:0,
                    from Documentation/networking/timestamping/timestamping.c:46:
>> ./usr/include/linux/time.h:26:8: error: redefinition of 'struct timezone'
    struct timezone {
           ^~~~~~~~
   In file included from Documentation/networking/timestamping/timestamping.c:37:0:
   /usr/include/x86_64-linux-gnu/sys/time.h:55:8: note: originally defined here
    struct timezone
           ^~~~~~~~
--
   In file included from ./usr/include/linux/errqueue.h:4:0,
                    from Documentation/networking/timestamping/txtimestamp.c:40:
>> ./usr/include/linux/time.h:19:8: error: redefinition of 'struct timeval'
    struct timeval {
           ^~~~~~~
   In file included from /usr/include/x86_64-linux-gnu/sys/select.h:45:0,
                    from /usr/include/x86_64-linux-gnu/sys/types.h:219,
                    from /usr/include/x86_64-linux-gnu/sys/uio.h:23,
                    from /usr/include/x86_64-linux-gnu/sys/socket.h:26,
                    from /usr/include/netinet/in.h:23,
                    from /usr/include/arpa/inet.h:22,
                    from Documentation/networking/timestamping/txtimestamp.c:35:
   /usr/include/x86_64-linux-gnu/bits/time.h:30:8: note: originally defined here
    struct timeval
           ^~~~~~~
   In file included from Documentation/networking/timestamping/txtimestamp.c:59:0:
>> /usr/include/x86_64-linux-gnu/sys/time.h:55:8: error: redefinition of 'struct timezone'
    struct timezone
           ^~~~~~~~
   In file included from ./usr/include/linux/errqueue.h:4:0,
                    from Documentation/networking/timestamping/txtimestamp.c:40:
   ./usr/include/linux/time.h:26:8: note: originally defined here
    struct timezone {
           ^~~~~~~~
>> ./usr/include/linux/time.h:38:22: error: expected identifier before numeric constant
    #define ITIMER_REAL  0
                         ^
   In file included from Documentation/networking/timestamping/txtimestamp.c:59:0:
>> /usr/include/x86_64-linux-gnu/sys/time.h:107:8: error: redefinition of 'struct itimerval'
    struct itimerval
           ^~~~~~~~~
   In file included from ./usr/include/linux/errqueue.h:4:0,
                    from Documentation/networking/timestamping/txtimestamp.c:40:
   ./usr/include/linux/time.h:51:8: note: originally defined here
    struct itimerval {
           ^~~~~~~~~
   In file included from Documentation/networking/timestamping/txtimestamp.c:61:0:
>> /usr/include/time.h:161:8: error: redefinition of 'struct itimerspec'
    struct itimerspec
           ^~~~~~~~~~
   In file included from ./usr/include/linux/errqueue.h:4:0,
                    from Documentation/networking/timestamping/txtimestamp.c:40:
   ./usr/include/linux/time.h:44:8: note: originally defined here
    struct itimerspec {
           ^~~~~~~~~~

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 25070 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH net-next 2/2] errqueue: include linux/time.h
  2016-09-12 19:26   ` kbuild test robot
@ 2016-09-12 20:09     ` Willem de Bruijn
  0 siblings, 0 replies; 5+ messages in thread
From: Willem de Bruijn @ 2016-09-12 20:09 UTC (permalink / raw)
  To: kbuild test robot
  Cc: kbuild-all, Network Development, David Miller, LKML, John Stultz,
	Brooks Moses, Willem de Bruijn

On Mon, Sep 12, 2016 at 3:26 PM, kbuild test robot <lkp@intel.com> wrote:
> Hi Willem,
>
> [auto build test ERROR on net-next/master]
>
> url:    https://github.com/0day-ci/linux/commits/Willem-de-Bruijn/uapi-include-time-h-from-errqueue-h/20160913-020431
> config: i386-defconfig (attached as .config)
> compiler: gcc-6 (Debian 6.1.1-9) 6.1.1 20160705
> reproduce:
>         # save the attached .config to linux build tree
>         make ARCH=i386
>
> All errors (new ones prefixed by >>):

This error report shows that breakage can occur with applications that
include linux/errqueue.h before the libc time headers.

The libc-compat definitions in this patch set only fix compilation
when uapi headers are included after the userspace headers.

These errors indeed go away when errqueue.h is included after the
userspace time includes, as in the diff below.

For the inverse, the libc headers need additional #if __UAPI_DEF_FOO
changes, as described in include/uapli/linux/libc-compat.h. Those
changes are a noop without kernel definitions, so arguably that libc
patch should be merged before this kernel patch.

I will remove this patch set from the patchwork queue for now.

diff --git a/Documentation/networking/timestamping/txtimestamp.c
b/Documentation/networking/timestamping/txtimestamp.c
index 5df0704..f073801 100644
--- a/Documentation/networking/timestamping/txtimestamp.c
+++ b/Documentation/networking/timestamping/txtimestamp.c
@@ -37,7 +37,6 @@
 #include <error.h>
 #include <errno.h>
 #include <inttypes.h>
-#include <linux/errqueue.h>
 #include <linux/if_ether.h>
 #include <linux/net_tstamp.h>
 #include <netdb.h>
@@ -59,6 +58,7 @@
 #include <sys/time.h>
 #include <sys/types.h>
 #include <time.h>
+#include <linux/errqueue.h>
 #include <unistd.h>

^ permalink raw reply related	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-09-12 20:10 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-12 17:05 [PATCH net-next 0/2] uapi: include time.h from errqueue.h Willem de Bruijn
2016-09-12 17:05 ` [PATCH net-next 1/2] uapi glibc compat: make linux/time.h compile with user time.h files Willem de Bruijn
2016-09-12 17:05 ` [PATCH net-next 2/2] errqueue: include linux/time.h Willem de Bruijn
2016-09-12 19:26   ` kbuild test robot
2016-09-12 20:09     ` Willem de Bruijn

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).