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

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Fri, Jun 14, 2019 at 7:27 AM Carlos O'Donell <carlos@redhat.com> wrote:
>
> 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"
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This may have caused:

https://sourceware.org/bugzilla/show_bug.cgi?id=24752

> >  # ifdef STATX_TYPE
> >  #  define __statx_timestamp_defined 1
> >  #  define __statx_defined 1
> >
>
>
> --
> Cheers,
> Carlos.



-- 
H.J.