Message ID | 20180621064654.2D7DA4289B0D0@oldenburg.str.redhat.com |
---|---|
State | New |
Headers | show |
Series | time: Define _STRUCT_TIMESPEC in <bits/types/struct_timespec.h> | expand |
On Thu, Jun 21, 2018 at 2:46 AM, Florian Weimer <fweimer@redhat.com> wrote: > After commit d76d3703551a362b472c866b5b6089f66f8daa8e ("Fix missing > timespec definition for sys/stat.h (BZ #21371)"), sanitizer builds start > to fail due to a conflicting definition of struct timespec in > <linux/time.h>. Define _STRUCT_TIMESPEC, which is already checked in > the kernel header, to support including <linux/time.h> after > <sys/stat.h>. Should it go the other way around as well? That is, if _STRUCT_TIMESPEC is already defined, should we suppress our definition? Either way I think there should be a comment saying that linux/time.h checks this macro. zw
On 06/21/2018 01:00 PM, Zack Weinberg wrote: > On Thu, Jun 21, 2018 at 2:46 AM, Florian Weimer <fweimer@redhat.com> wrote: >> After commit d76d3703551a362b472c866b5b6089f66f8daa8e ("Fix missing >> timespec definition for sys/stat.h (BZ #21371)"), sanitizer builds start >> to fail due to a conflicting definition of struct timespec in >> <linux/time.h>. Define _STRUCT_TIMESPEC, which is already checked in >> the kernel header, to support including <linux/time.h> after >> <sys/stat.h>. > > Should it go the other way around as well? That is, if > _STRUCT_TIMESPEC is already defined, should we suppress our > definition? Hmm, sure, that would be possible. > Either way I think there should be a comment saying that linux/time.h > checks this macro. It's in generic code, so I wasn't sure if it was okay to refer to <linux/time.h>. But I can certainly add that. What about the attached patch? Thanks, Florian Subject: [PATCH] time: Define _STRUCT_TIMESPEC in <bits/types/struct_timespec.h> To: libc-alpha@sourceware.org After commit d76d3703551a362b472c866b5b6089f66f8daa8e ("Fix missing timespec definition for sys/stat.h (BZ #21371)"), sanitizer builds start to fail due to a conflicting definition of struct timespec in <linux/time.h>. Use _STRUCT_TIMESPEC as the header file inclusion guard, which is already checked in the kernel header, to support including <linux/time.h> and <sys/stat.h> in the same translation unit. 2018-06-21 Florian Weimer <fweimer@redhat.com> * time/bits/types/struct_timespec.h (_STRUCT_TIMESPEC): Define. diff --git a/time/bits/types/struct_timespec.h b/time/bits/types/struct_timespec.h index 644db9fdb6..5b77c52b4f 100644 --- a/time/bits/types/struct_timespec.h +++ b/time/bits/types/struct_timespec.h @@ -1,5 +1,6 @@ -#ifndef __timespec_defined -#define __timespec_defined 1 +/* NB: Include guard matches what <linux/time.h> uses. */ +#ifndef _STRUCT_TIMESPEC +#define _STRUCT_TIMESPEC 1 #include <bits/types.h>
On 06/21/2018 01:24 PM, Florian Weimer wrote: > 2018-06-21 Florian Weimer<fweimer@redhat.com> > > * time/bits/types/struct_timespec.h (_STRUCT_TIMESPEC): Define. > > diff --git a/time/bits/types/struct_timespec.h b/time/bits/types/struct_timespec.h > index 644db9fdb6..5b77c52b4f 100644 > --- a/time/bits/types/struct_timespec.h > +++ b/time/bits/types/struct_timespec.h > @@ -1,5 +1,6 @@ > -#ifndef __timespec_defined > -#define __timespec_defined 1 > +/* NB: Include guard matches what <linux/time.h> uses. */ > +#ifndef _STRUCT_TIMESPEC > +#define _STRUCT_TIMESPEC 1 > > #include <bits/types.h> Ping? Thanks, Florian
On 06/21/2018 01:24 PM, Florian Weimer wrote: > diff --git a/time/bits/types/struct_timespec.h b/time/bits/types/struct_timespec.h > index 644db9fdb6..5b77c52b4f 100644 > --- a/time/bits/types/struct_timespec.h > +++ b/time/bits/types/struct_timespec.h > @@ -1,5 +1,6 @@ > -#ifndef __timespec_defined > -#define __timespec_defined 1 > +/* NB: Include guard matches what <linux/time.h> uses. */ > +#ifndef _STRUCT_TIMESPEC > +#define _STRUCT_TIMESPEC 1 Are there any objections to this change? It's required to fix a GCC build failure, so I'm going to commit this soon (probably on Friday). Thanks, Florian
On Thu, Jun 28, 2018 at 10:27:53AM +0200, Florian Weimer wrote: > On 06/21/2018 01:24 PM, Florian Weimer wrote: > > diff --git a/time/bits/types/struct_timespec.h b/time/bits/types/struct_timespec.h > > index 644db9fdb6..5b77c52b4f 100644 > > --- a/time/bits/types/struct_timespec.h > > +++ b/time/bits/types/struct_timespec.h > > @@ -1,5 +1,6 @@ > > -#ifndef __timespec_defined > > -#define __timespec_defined 1 > > +/* NB: Include guard matches what <linux/time.h> uses. */ > > +#ifndef _STRUCT_TIMESPEC > > +#define _STRUCT_TIMESPEC 1 > > Are there any objections to this change? It's required to fix a GCC > build failure, so I'm going to commit this soon (probably on Friday). The change looks fine, thanks.
On 06/28/2018 11:52 AM, Dmitry V. Levin wrote: > On Thu, Jun 28, 2018 at 10:27:53AM +0200, Florian Weimer wrote: >> On 06/21/2018 01:24 PM, Florian Weimer wrote: >>> diff --git a/time/bits/types/struct_timespec.h b/time/bits/types/struct_timespec.h >>> index 644db9fdb6..5b77c52b4f 100644 >>> --- a/time/bits/types/struct_timespec.h >>> +++ b/time/bits/types/struct_timespec.h >>> @@ -1,5 +1,6 @@ >>> -#ifndef __timespec_defined >>> -#define __timespec_defined 1 >>> +/* NB: Include guard matches what <linux/time.h> uses. */ >>> +#ifndef _STRUCT_TIMESPEC >>> +#define _STRUCT_TIMESPEC 1 >> >> Are there any objections to this change? It's required to fix a GCC >> build failure, so I'm going to commit this soon (probably on Friday). > > The change looks fine, thanks. Thanks. I filed bug 23349 to track this and will reference it in the commit. Florian
diff --git a/time/bits/types/struct_timespec.h b/time/bits/types/struct_timespec.h index 644db9fdb6..bde7e2826d 100644 --- a/time/bits/types/struct_timespec.h +++ b/time/bits/types/struct_timespec.h @@ -1,6 +1,10 @@ #ifndef __timespec_defined #define __timespec_defined 1 +#ifndef _STRUCT_TIMESPEC +# define _STRUCT_TIMESPEC 1 +#endif + #include <bits/types.h> /* POSIX.1b structure for a time value. This is like a `struct timeval' but