This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [glibc] <sys/stat.h>: Use Linux UAPI header for statx if available and useful
On 6/14/19 10:22 AM, Florian Weimer wrote:
> * Szabolcs Nagy:
>
>> 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.
>
> Thanks. Updated patch below. I plan to push this soon.
>
> 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-14 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.
LGTM. FYI, Rich Felker tweeted about this :-)
Reviewed-by: Carlos O'Donell <carlos@redhat.com>
> diff --git a/sysdeps/unix/sysv/linux/bits/statx.h b/sysdeps/unix/sysv/linux/bits/statx.h
> index d36f44efc6..206878723f 100644
> --- a/sysdeps/unix/sysv/linux/bits/statx.h
> +++ b/sysdeps/unix/sysv/linux/bits/statx.h
> @@ -23,8 +23,11 @@
> #endif
>
> /* Use the Linux kernel header if available. */
> -#if __glibc_has_include (<linux/stat.h>)
> -# include <linux/stat.h>
> +
> +/* Use "" to work around incorrect macro expansion of the
> + __has_include argument (GCC PR 80005). */
> +#if __glibc_has_include ("linux/stat.h")
> +# include "linux/stat.h"
> # ifdef STATX_TYPE
> # define __statx_timestamp_defined 1
> # define __statx_defined 1
>
--
Cheers,
Carlos.