Message ID | 87ef3y6bpv.fsf@oldenburg2.str.redhat.com |
---|---|
State | Superseded |
Headers |
Received: (qmail 119489 invoked by alias); 12 Jun 2019 15:54:24 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <libc-alpha.sourceware.org> List-Unsubscribe: <mailto:libc-alpha-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:libc-alpha-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 119453 invoked by uid 89); 12 Jun 2019 15:54:24 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-19.0 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, SPF_HELO_PASS autolearn=ham version=3.3.1 spammy=HX-Languages-Length:1457, H*i:sk:c35c19d, H*f:sk:c35c19d X-HELO: mx1.redhat.com From: Florian Weimer <fweimer@redhat.com> To: Szabolcs Nagy <Szabolcs.Nagy@arm.com> Cc: Zack Weinberg <zackw@panix.com>, nd <nd@arm.com>, GNU C Library <libc-alpha@sourceware.org> Subject: Re: [glibc] <sys/stat.h>: Use Linux UAPI header for statx if available and useful References: <20190612110504.66041.qmail@sourceware.org> <87ftoe7utd.fsf@oldenburg2.str.redhat.com> <CAKCAbMj2G7_1bAuE+5X0qvNGMTMN9QenzOpgJmkwC4LK_mrGzA@mail.gmail.com> <87y3266czw.fsf@oldenburg2.str.redhat.com> <c35c19da-b775-3435-d38e-702f30bc6ac3@arm.com> Date: Wed, 12 Jun 2019 17:54:20 +0200 In-Reply-To: <c35c19da-b775-3435-d38e-702f30bc6ac3@arm.com> (Szabolcs Nagy's message of "Wed, 12 Jun 2019 15:31:39 +0000") Message-ID: <87ef3y6bpv.fsf@oldenburg2.str.redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain |
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
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 >
* 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
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 >
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