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

Message ID 87ef3y6bpv.fsf@oldenburg2.str.redhat.com
State Superseded
Headers

Commit Message

Florian Weimer June 12, 2019, 3:54 p.m. UTC
  * Szabolcs Nagy:

> ouch, use the ""
>
> (clang __has_include seems not to expand)

Yes, that's what I saw as well in my testing.  Below is a lightly tested
patch.

Thanks,
Florian

Linux: Fix __glibc_has_include use for <sys/stat.h> and statx

The identifier linux is used as a predefined macro, so the actually used
path is 1/stat.h or 1/stat64.h.  Using the quote-based version triggers
a file lookup for /usr/include/bits/linux/stat.h (or whatever directory
is used to store bits/statx.h), but since bits/ is pretty much reserved
by glibc, this appears to be acceptable.

This is related to GCC PR 80005: incorrect macro expansion of the
argument of __has_include.

Suggested by Zack Weinberg.

2019-06-12  Florian Weimer  <fweimer@redhat.com>

	* sysdeps/unix/sysv/linux/bits/statx.h: Use string literal in
	argument to __glibc_has_include to inhibit macro expansion.
  

Comments

Szabolcs Nagy June 12, 2019, 4 p.m. UTC | #1
On 12/06/2019 16:54, Florian Weimer wrote:
> Linux: Fix __glibc_has_include use for <sys/stat.h> and statx

> 

> The identifier linux is used as a predefined macro, so the actually used

> path is 1/stat.h or 1/stat64.h.  Using the quote-based version triggers

> a file lookup for /usr/include/bits/linux/stat.h (or whatever directory

> is used to store bits/statx.h), but since bits/ is pretty much reserved

> by glibc, this appears to be acceptable.

> 

> This is related to GCC PR 80005: incorrect macro expansion of the

> argument of __has_include.

> 

> Suggested by Zack Weinberg.

> 

> 2019-06-12  Florian Weimer  <fweimer@redhat.com>

> 

> 	* sysdeps/unix/sysv/linux/bits/statx.h: Use string literal in

> 	argument to __glibc_has_include to inhibit macro expansion.

> 

> diff --git a/sysdeps/unix/sysv/linux/bits/statx.h b/sysdeps/unix/sysv/linux/bits/statx.h

> index d36f44efc6..3599f85a47 100644

> --- a/sysdeps/unix/sysv/linux/bits/statx.h

> +++ b/sysdeps/unix/sysv/linux/bits/statx.h

> @@ -23,8 +23,8 @@

>  #endif

>  

>  /* Use the Linux kernel header if available.  */

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

> -# include <linux/stat.h>

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

> +# include "linux/stat.h"


i would add a comment here with the gcc bug number as a
reminder in case anybody wonders about ""

>  # ifdef STATX_TYPE

>  #  define __statx_timestamp_defined 1

>  #  define __statx_defined 1

>
  
Florian Weimer June 12, 2019, 4:03 p.m. UTC | #2
* Szabolcs Nagy:

> On 12/06/2019 16:54, Florian Weimer wrote:
>> Linux: Fix __glibc_has_include use for <sys/stat.h> and statx
>> 
>> The identifier linux is used as a predefined macro, so the actually used
>> path is 1/stat.h or 1/stat64.h.  Using the quote-based version triggers
>> a file lookup for /usr/include/bits/linux/stat.h (or whatever directory
>> is used to store bits/statx.h), but since bits/ is pretty much reserved
>> by glibc, this appears to be acceptable.
>> 
>> This is related to GCC PR 80005: incorrect macro expansion of the
>> argument of __has_include.
>> 
>> Suggested by Zack Weinberg.
>> 
>> 2019-06-12  Florian Weimer  <fweimer@redhat.com>
>> 
>> 	* sysdeps/unix/sysv/linux/bits/statx.h: Use string literal in
>> 	argument to __glibc_has_include to inhibit macro expansion.
>> 
>> diff --git a/sysdeps/unix/sysv/linux/bits/statx.h b/sysdeps/unix/sysv/linux/bits/statx.h
>> index d36f44efc6..3599f85a47 100644
>> --- a/sysdeps/unix/sysv/linux/bits/statx.h
>> +++ b/sysdeps/unix/sysv/linux/bits/statx.h
>> @@ -23,8 +23,8 @@
>>  #endif
>>  
>>  /* Use the Linux kernel header if available.  */
>> -#if __glibc_has_include (<linux/stat.h>)
>> -# include <linux/stat.h>
>> +#if __glibc_has_include ("linux/stat.h")
>> +# include "linux/stat.h"
>
> i would add a comment here with the gcc bug number as a
> reminder in case anybody wonders about ""

What about this?

/* Use "" to work around incorrect macro expansion of the __has_include
   argument (GCC PR 80005).  */

This will be correctly only with the other patch eliminating the
function-like macro (just posted on a new thread).

Thanks,
Florian
  
Szabolcs Nagy June 12, 2019, 4:05 p.m. UTC | #3
On 12/06/2019 17:03, Florian Weimer wrote:
> * Szabolcs Nagy:

> 

>> On 12/06/2019 16:54, Florian Weimer wrote:

>>> Linux: Fix __glibc_has_include use for <sys/stat.h> and statx

>>>

>>> The identifier linux is used as a predefined macro, so the actually used

>>> path is 1/stat.h or 1/stat64.h.  Using the quote-based version triggers

>>> a file lookup for /usr/include/bits/linux/stat.h (or whatever directory

>>> is used to store bits/statx.h), but since bits/ is pretty much reserved

>>> by glibc, this appears to be acceptable.

>>>

>>> This is related to GCC PR 80005: incorrect macro expansion of the

>>> argument of __has_include.

>>>

>>> Suggested by Zack Weinberg.

>>>

>>> 2019-06-12  Florian Weimer  <fweimer@redhat.com>

>>>

>>> 	* sysdeps/unix/sysv/linux/bits/statx.h: Use string literal in

>>> 	argument to __glibc_has_include to inhibit macro expansion.

>>>

>>> diff --git a/sysdeps/unix/sysv/linux/bits/statx.h b/sysdeps/unix/sysv/linux/bits/statx.h

>>> index d36f44efc6..3599f85a47 100644

>>> --- a/sysdeps/unix/sysv/linux/bits/statx.h

>>> +++ b/sysdeps/unix/sysv/linux/bits/statx.h

>>> @@ -23,8 +23,8 @@

>>>  #endif

>>>  

>>>  /* Use the Linux kernel header if available.  */

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

>>> -# include <linux/stat.h>

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

>>> +# include "linux/stat.h"

>>

>> i would add a comment here with the gcc bug number as a

>> reminder in case anybody wonders about ""

> 

> What about this?

> 

> /* Use "" to work around incorrect macro expansion of the __has_include

>    argument (GCC PR 80005).  */


looks good.

> 

> This will be correctly only with the other patch eliminating the

> function-like macro (just posted on a new thread).

> 

> Thanks,

> Florian

>
  

Patch

diff --git a/sysdeps/unix/sysv/linux/bits/statx.h b/sysdeps/unix/sysv/linux/bits/statx.h
index d36f44efc6..3599f85a47 100644
--- a/sysdeps/unix/sysv/linux/bits/statx.h
+++ b/sysdeps/unix/sysv/linux/bits/statx.h
@@ -23,8 +23,8 @@ 
 #endif
 
 /* Use the Linux kernel header if available.  */
-#if __glibc_has_include (<linux/stat.h>)
-# include <linux/stat.h>
+#if __glibc_has_include ("linux/stat.h")
+# include "linux/stat.h"
 # ifdef STATX_TYPE
 #  define __statx_timestamp_defined 1
 #  define __statx_defined 1