diff mbox

<sys/stat.h>: Use Linux UAPI header for statx if available and useful

Message ID 87y3266czw.fsf@oldenburg2.str.redhat.com
State Dropped
Headers show

Commit Message

Florian Weimer June 12, 2019, 3:26 p.m. UTC
* Zack Weinberg:

> On Wed, Jun 12, 2019 at 10:16 AM Florian Weimer <fweimer@redhat.com> wrote:
>>
>> > +#if __glibc_has_include (<linux/stat.h>)
> ...
>>
>> It turns out that this does not work as expected in some configurations
>> because linux is a macro, defined to 1, and it's expanded in this
>> context
>
> I wonder how much stuff still depends on these legacy user-namespace
> OS-specific macros.  Would anyone even notice if we put
>
> #undef linux
> #undef unix
>
> at the top of features.h?  The GCC manual has said "We are slowly
> phasing out all predefined macros which are outside the reserved
> namespace" [1] since, um, 2001 give or take? but I don't think anyone
> has ever done any actual work on getting rid of them :-/

It's a bit drastic for this change.  I want to backport it (at last
downstream), after all.

>> Is there a way to inhibit macro expansion in this context?  Maybe with
>> token pasting?
>
> The easiest thing that comes to mind is to write __glibc_has_include
> ("linux/stat.h") instead. Then the preprocessor would see the argument
> as a string literal and not expand "linux", and there shouldn't be a
> /usr/include/sys/linux/stat.h ever, so it would come out equivalently.
> Well, I guess someone could be using -iquote, but do we really need to
> support the situation where someone pointed -iquote at a directory
> containing a linux/stat.h, and at the same time there's no
> linux/stat.h in the <> search path?

Would that approach be acceptable?  Then let's use that.

It also avoids the same problem for the stat token.

Alternatively, we would have to use something like this:


But this is a bit iffy because there's no guarantee that linux == 1.

Thanks,
Florian

Comments

Szabolcs Nagy June 12, 2019, 3:31 p.m. UTC | #1
On 12/06/2019 16:26, Florian Weimer wrote:
> * Zack Weinberg:

>> On Wed, Jun 12, 2019 at 10:16 AM Florian Weimer <fweimer@redhat.com> wrote:

>>>

>>>> +#if __glibc_has_include (<linux/stat.h>)

>> ...

>>>

>>> It turns out that this does not work as expected in some configurations

>>> because linux is a macro, defined to 1, and it's expanded in this

>>> context

>>

>> I wonder how much stuff still depends on these legacy user-namespace

>> OS-specific macros.  Would anyone even notice if we put

>>

>> #undef linux

>> #undef unix

>>

>> at the top of features.h?  The GCC manual has said "We are slowly

>> phasing out all predefined macros which are outside the reserved

>> namespace" [1] since, um, 2001 give or take? but I don't think anyone

>> has ever done any actual work on getting rid of them :-/

> 

> It's a bit drastic for this change.  I want to backport it (at last

> downstream), after all.


gcc should first start poisoning all uses of these macros.

then a release later remove them.

> 

>>> Is there a way to inhibit macro expansion in this context?  Maybe with

>>> token pasting?

>>

>> The easiest thing that comes to mind is to write __glibc_has_include

>> ("linux/stat.h") instead. Then the preprocessor would see the argument

>> as a string literal and not expand "linux", and there shouldn't be a

>> /usr/include/sys/linux/stat.h ever, so it would come out equivalently.

>> Well, I guess someone could be using -iquote, but do we really need to

>> support the situation where someone pointed -iquote at a directory

>> containing a linux/stat.h, and at the same time there's no

>> linux/stat.h in the <> search path?

> 

> Would that approach be acceptable?  Then let's use that.

> 

> It also avoids the same problem for the stat token.

> 

> Alternatively, we would have to use something like this:


ouch, use the ""

(clang __has_include seems not to expand)
diff mbox

Patch

diff --git a/io/sys/stat.h b/io/sys/stat.h
index 2de5eb65d9..aba049f6c8 100644
--- a/io/sys/stat.h
+++ b/io/sys/stat.h
@@ -368,6 +368,10 @@  extern int utimensat (int __fd, const char *__path,
 extern int futimens (int __fd, const struct timespec __times[2]) __THROW;
 #endif
 
+#ifdef __USE_GNU
+# include <bits/statx.h>
+#endif
+
 /* To allow the `struct stat' structure and the file type `mode_t'
    bits to vary without changing shared library major version number,
    the `stat' family of functions and `mknod' are in fact inline
@@ -442,10 +446,6 @@  extern int __xmknodat (int __ver, int __fd, const char *__path,
 		       __mode_t __mode, __dev_t *__dev)
      __THROW __nonnull ((3, 5));
 
-#ifdef __USE_GNU
-# include <bits/statx.h>
-#endif
-
 #ifdef __USE_EXTERN_INLINES
 /* Inlined versions of the real stat and mknod functions.  */
 
diff --git a/sysdeps/unix/sysv/linux/bits/statx.h b/sysdeps/unix/sysv/linux/bits/statx.h
index d36f44efc6..168001a216 100644
--- a/sysdeps/unix/sysv/linux/bits/statx.h
+++ b/sysdeps/unix/sysv/linux/bits/statx.h
@@ -22,13 +22,26 @@ 
 # error Never include <bits/statx.h> directly, include <sys/stat.h> instead.
 #endif
 
-/* Use the Linux kernel header if available.  */
-#if __glibc_has_include (<linux/stat.h>)
-# include <linux/stat.h>
-# ifdef STATX_TYPE
-#  define __statx_timestamp_defined 1
-#  define __statx_defined 1
+
+#ifdef __has_include
+/* Work around GCC PR 80005.  */
+# ifdef linux
+#  define __glibc_linux_defined
+#  undef linux
 # endif
-#endif
+
+/* Use the Linux kernel header if available.  */
+# if __has_include (<linux/stat.h>)
+#  include <linux/stat.h>
+#  ifdef STATX_TYPE
+#   define __statx_timestamp_defined 1
+#   define __statx_defined 1
+#  endif
+# endif /* <linux/stat.h> */
+# ifdef __glibc_linux_defined
+#  define linux 1
+#  undef __glibc_linux_defined
+# endif /* linux was defined originally.  */
+#endif /* __has_include is available.  */
 
 #include <bits/statx-generic.h>