Message ID | 20220207201058.250114-1-dimitar@dinux.eu |
---|---|
State | Superseded |
Headers |
Return-Path: <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 44316385841C for <patchwork@sourceware.org>; Mon, 7 Feb 2022 20:11:45 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from server28.superhosting.bg (server28.superhosting.bg [217.174.156.11]) by sourceware.org (Postfix) with ESMTPS id B768B3858C83; Mon, 7 Feb 2022 20:11:26 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org B768B3858C83 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=dinux.eu Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=dinux.eu DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=dinux.eu; s=default; h=Content-Transfer-Encoding:Content-Type:MIME-Version:Message-Id: Date:Subject:Cc:To:From:Sender:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: In-Reply-To:References:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=Pzo36LFt2Z4gtViM4vE9fJtIGwcbic4Ql16FjVOIgKA=; b=l3aNyypqIul+sb19LcvjrUr4Xr uo3M6zZcGioWAsH/rrO2RIKoydX8ZjseGYymcHktXRb1/j2Akzf/7ygDssJHWHnyY52EE5bH3Ms9I J5kf3UnPj6ryL5q0nMV5r3M9GYR1AaOO526yjqUmxhUDsPOHFfDh6kQCigMtZ4S1dsgB+HFhKH+YV CLxF5Q+xS2UrjAYlSi4kFCoiljV20OHdcfmZq76go7zCSG+cYK6dtqVW6/xdKFx+cvzPjFjydBPzz ResrsPgyux/JoBNma9xa3Jjd4lANz2gZTnMtCFsDjSK/8qcmjDuTgaxViJ9wjtygH/QGonrhfUj6k W3YidPOw==; Received: from [95.87.234.74] (port=52398 helo=kendros.lan) by server28.superhosting.bg with esmtpsa (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from <dimitar@dinux.eu>) id 1nHAM4-000AUT-WF; Mon, 07 Feb 2022 22:11:24 +0200 From: Dimitar Dimitrov <dimitar@dinux.eu> To: libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org Subject: [PATCH] libstdc++: Decouple HAVE_FCNTL_H from HAVE_DIRENT_H check Date: Mon, 7 Feb 2022 22:10:58 +0200 Message-Id: <20220207201058.250114-1-dimitar@dinux.eu> X-Mailer: git-send-email 2.34.1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-OutGoing-Spam-Status: No, score=-2.9 X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - server28.superhosting.bg X-AntiAbuse: Original Domain - gcc.gnu.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - dinux.eu X-Get-Message-Sender-Via: server28.superhosting.bg: authenticated_id: dimitar@dinux.eu X-Authenticated-Sender: server28.superhosting.bg: dimitar@dinux.eu X-Source: X-Source-Args: X-Source-Dir: X-Spam-Status: No, score=-11.3 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, HAS_X_OUTGOING_SPAM_STAT, RCVD_IN_DNSWL_NONE, SPF_HELO_PASS, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list <gcc-patches.gcc.gnu.org> List-Unsubscribe: <https://gcc.gnu.org/mailman/options/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=unsubscribe> List-Archive: <https://gcc.gnu.org/pipermail/gcc-patches/> List-Post: <mailto:gcc-patches@gcc.gnu.org> List-Help: <mailto:gcc-patches-request@gcc.gnu.org?subject=help> List-Subscribe: <https://gcc.gnu.org/mailman/listinfo/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=subscribe> Cc: Jonathan Wakely <jwakely@redhat.com> Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> |
Series |
libstdc++: Decouple HAVE_FCNTL_H from HAVE_DIRENT_H check
|
|
Commit Message
Dimitar Dimitrov
Feb. 7, 2022, 8:10 p.m. UTC
On PRU target with newlib, we have the following combination in config.h:
/* #undef HAVE_DIRENT_H */
#define HAVE_FCNTL_H 1
#define HAVE_UNLINKAT 1
In newlib, targets which do not define dirent.h, get a build error when
including <dirent.h>:
https://sourceware.org/git/?p=newlib-cygwin.git;a=blob;f=newlib/libc/include/sys/dirent.h;hb=HEAD
While fs_dir.cc correctly checks for HAVE_FCNTL_H, dir-common.h doesn't,
and instead uses HAVE_DIRENT_H. This results in unlinkat() function call
in fs_dir.cc without the needed <fcntl.h> include in dir-common.h. Thus
a build failure:
.../gcc/libstdc++-v3/src/c++17/fs_dir.cc:151:11: error: ‘::unlinkat’ has not been declared; did you mean ‘unlink’?
Fix by encapsulating <fcntl.h> include with the correct check.
Regtested x86_64-pc-linux-gnu and no new failures detected.
Fixes commit r12-7062-gebf61754647689 (libstdc++: Fix filesystem::remove_all races).
libstdc++-v3/ChangeLog:
* src/filesystem/dir-common.h (_GLIBCXX_HAVE_FCNTL_H): Move the
check outside the HAVE_DIRENT_H check.
Signed-off-by: Dimitar Dimitrov <dimitar@dinux.eu>
---
libstdc++-v3/src/filesystem/dir-common.h | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
Comments
On Mon, 7 Feb 2022 at 20:12, Dimitar Dimitrov <dimitar@dinux.eu> wrote: > On PRU target with newlib, we have the following combination in config.h: > /* #undef HAVE_DIRENT_H */ > #define HAVE_FCNTL_H 1 > #define HAVE_UNLINKAT 1 > > In newlib, targets which do not define dirent.h, get a build error when > including <dirent.h>: > > https://sourceware.org/git/?p=newlib-cygwin.git;a=blob;f=newlib/libc/include/sys/dirent.h;hb=HEAD > > While fs_dir.cc correctly checks for HAVE_FCNTL_H, dir-common.h doesn't, > and instead uses HAVE_DIRENT_H. This results in unlinkat() function call > in fs_dir.cc without the needed <fcntl.h> include in dir-common.h. Thus > a build failure: > .../gcc/libstdc++-v3/src/c++17/fs_dir.cc:151:11: error: ‘::unlinkat’ has > not been declared; did you mean ‘unlink’? > > Fix by encapsulating <fcntl.h> include with the correct check. > But there's no point doing anything in that file if we don't have <dirent.h>, the whole thing is unusable. There's no point making the members using unlinkat compile if you can't ever construct the type. So I think we want a different fix. > Regtested x86_64-pc-linux-gnu and no new failures detected. > > Fixes commit r12-7062-gebf61754647689 (libstdc++: Fix > filesystem::remove_all races). > > libstdc++-v3/ChangeLog: > > * src/filesystem/dir-common.h (_GLIBCXX_HAVE_FCNTL_H): Move the > check outside the HAVE_DIRENT_H check. > > Signed-off-by: Dimitar Dimitrov <dimitar@dinux.eu> > --- > libstdc++-v3/src/filesystem/dir-common.h | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/libstdc++-v3/src/filesystem/dir-common.h > b/libstdc++-v3/src/filesystem/dir-common.h > index 0b7665a3f70..cfce4fae9a4 100644 > --- a/libstdc++-v3/src/filesystem/dir-common.h > +++ b/libstdc++-v3/src/filesystem/dir-common.h > @@ -35,10 +35,11 @@ > # include <sys/types.h> > # endif > # include <dirent.h> // opendir, readdir, fdopendir, dirfd > -# ifdef _GLIBCXX_HAVE_FCNTL_H > -# include <fcntl.h> // open, openat, fcntl, AT_FDCWD, O_NOFOLLOW etc. > -# include <unistd.h> // close, unlinkat > -# endif > +#endif > + > +#ifdef _GLIBCXX_HAVE_FCNTL_H > +# include <fcntl.h> // open, openat, fcntl, AT_FDCWD, O_NOFOLLOW etc. > +# include <unistd.h> // close, unlinkat > #endif > > namespace std _GLIBCXX_VISIBILITY(default) > -- > 2.34.1 > >
On Mon, 7 Feb 2022 at 21:01, Jonathan Wakely <jwakely.gcc@gmail.com> wrote: > > > On Mon, 7 Feb 2022 at 20:12, Dimitar Dimitrov <dimitar@dinux.eu> wrote: > >> On PRU target with newlib, we have the following combination in config.h: >> /* #undef HAVE_DIRENT_H */ >> #define HAVE_FCNTL_H 1 >> #define HAVE_UNLINKAT 1 >> >> In newlib, targets which do not define dirent.h, get a build error when >> including <dirent.h>: >> >> https://sourceware.org/git/?p=newlib-cygwin.git;a=blob;f=newlib/libc/include/sys/dirent.h;hb=HEAD >> >> While fs_dir.cc correctly checks for HAVE_FCNTL_H, dir-common.h doesn't, >> and instead uses HAVE_DIRENT_H. This results in unlinkat() function call >> in fs_dir.cc without the needed <fcntl.h> include in dir-common.h. Thus >> a build failure: >> .../gcc/libstdc++-v3/src/c++17/fs_dir.cc:151:11: error: ‘::unlinkat’ >> has not been declared; did you mean ‘unlink’? >> >> Fix by encapsulating <fcntl.h> include with the correct check. >> > > But there's no point doing anything in that file if we don't have > <dirent.h>, the whole thing is unusable. There's no point making the > members using unlinkat compile if you can't ever construct the type. > > So I think we want a different fix. > Maybe something like: --- a/libstdc++-v3/src/filesystem/dir-common.h +++ b/libstdc++-v3/src/filesystem/dir-common.h @@ -70,6 +70,8 @@ struct DIR { }; inline DIR* opendir(const char*) { return nullptr; } inline dirent* readdir(DIR*) { return nullptr; } inline int closedir(DIR*) { return -1; } +#undef _GLIBCXX_HAVE_DIRFD +#undef _GLIBCXX_HAVE_UNLINKAT #endif } // namespace __gnu_posix Or adding wrappers for those in namespace posix.
On Mon, Feb 07, 2022 at 09:05:45PM +0000, Jonathan Wakely wrote: > On Mon, 7 Feb 2022 at 21:01, Jonathan Wakely <jwakely.gcc@gmail.com> wrote: > > > > > > > On Mon, 7 Feb 2022 at 20:12, Dimitar Dimitrov <dimitar@dinux.eu> wrote: > > > >> On PRU target with newlib, we have the following combination in config.h: > >> /* #undef HAVE_DIRENT_H */ > >> #define HAVE_FCNTL_H 1 > >> #define HAVE_UNLINKAT 1 > >> > >> In newlib, targets which do not define dirent.h, get a build error when > >> including <dirent.h>: > >> > >> https://sourceware.org/git/?p=newlib-cygwin.git;a=blob;f=newlib/libc/include/sys/dirent.h;hb=HEAD > >> > >> While fs_dir.cc correctly checks for HAVE_FCNTL_H, dir-common.h doesn't, > >> and instead uses HAVE_DIRENT_H. This results in unlinkat() function call > >> in fs_dir.cc without the needed <fcntl.h> include in dir-common.h. Thus > >> a build failure: > >> .../gcc/libstdc++-v3/src/c++17/fs_dir.cc:151:11: error: ‘::unlinkat’ > >> has not been declared; did you mean ‘unlink’? > >> > >> Fix by encapsulating <fcntl.h> include with the correct check. > >> > > > > But there's no point doing anything in that file if we don't have > > <dirent.h>, the whole thing is unusable. There's no point making the > > members using unlinkat compile if you can't ever construct the type. > > > > So I think we want a different fix. > > > > > Maybe something like: > > --- a/libstdc++-v3/src/filesystem/dir-common.h > +++ b/libstdc++-v3/src/filesystem/dir-common.h > @@ -70,6 +70,8 @@ struct DIR { }; > inline DIR* opendir(const char*) { return nullptr; } > inline dirent* readdir(DIR*) { return nullptr; } > inline int closedir(DIR*) { return -1; } > +#undef _GLIBCXX_HAVE_DIRFD > +#undef _GLIBCXX_HAVE_UNLINKAT > #endif > } // namespace __gnu_posix Yes, this fixes the PRU target, and does not regress x86_64-pc-linux-gnu. Regards, Dimitar
On Tue, 8 Feb 2022 at 21:02, Dimitar Dimitrov <dimitar@dinux.eu> wrote: > On Mon, Feb 07, 2022 at 09:05:45PM +0000, Jonathan Wakely wrote: > > On Mon, 7 Feb 2022 at 21:01, Jonathan Wakely <jwakely.gcc@gmail.com> > wrote: > > > > > > > > > > > On Mon, 7 Feb 2022 at 20:12, Dimitar Dimitrov <dimitar@dinux.eu> > wrote: > > > > > >> On PRU target with newlib, we have the following combination in > config.h: > > >> /* #undef HAVE_DIRENT_H */ > > >> #define HAVE_FCNTL_H 1 > > >> #define HAVE_UNLINKAT 1 > > >> > > >> In newlib, targets which do not define dirent.h, get a build error > when > > >> including <dirent.h>: > > >> > > >> > https://sourceware.org/git/?p=newlib-cygwin.git;a=blob;f=newlib/libc/include/sys/dirent.h;hb=HEAD > > >> > > >> While fs_dir.cc correctly checks for HAVE_FCNTL_H, dir-common.h > doesn't, > > >> and instead uses HAVE_DIRENT_H. This results in unlinkat() function > call > > >> in fs_dir.cc without the needed <fcntl.h> include in dir-common.h. > Thus > > >> a build failure: > > >> .../gcc/libstdc++-v3/src/c++17/fs_dir.cc:151:11: error: ‘::unlinkat’ > > >> has not been declared; did you mean ‘unlink’? > > >> > > >> Fix by encapsulating <fcntl.h> include with the correct check. > > >> > > > > > > But there's no point doing anything in that file if we don't have > > > <dirent.h>, the whole thing is unusable. There's no point making the > > > members using unlinkat compile if you can't ever construct the type. > > > > > > So I think we want a different fix. > > > > > > > > > Maybe something like: > > > > --- a/libstdc++-v3/src/filesystem/dir-common.h > > +++ b/libstdc++-v3/src/filesystem/dir-common.h > > @@ -70,6 +70,8 @@ struct DIR { }; > > inline DIR* opendir(const char*) { return nullptr; } > > inline dirent* readdir(DIR*) { return nullptr; } > > inline int closedir(DIR*) { return -1; } > > +#undef _GLIBCXX_HAVE_DIRFD > > +#undef _GLIBCXX_HAVE_UNLINKAT > > #endif > > } // namespace __gnu_posix > Yes, this fixes the PRU target, and does not regress > x86_64-pc-linux-gnu. > Thanks for checking it. I'm just testing it myself on powerpc64le-linux-gnu and will push when it finishes.
diff --git a/libstdc++-v3/src/filesystem/dir-common.h b/libstdc++-v3/src/filesystem/dir-common.h index 0b7665a3f70..cfce4fae9a4 100644 --- a/libstdc++-v3/src/filesystem/dir-common.h +++ b/libstdc++-v3/src/filesystem/dir-common.h @@ -35,10 +35,11 @@ # include <sys/types.h> # endif # include <dirent.h> // opendir, readdir, fdopendir, dirfd -# ifdef _GLIBCXX_HAVE_FCNTL_H -# include <fcntl.h> // open, openat, fcntl, AT_FDCWD, O_NOFOLLOW etc. -# include <unistd.h> // close, unlinkat -# endif +#endif + +#ifdef _GLIBCXX_HAVE_FCNTL_H +# include <fcntl.h> // open, openat, fcntl, AT_FDCWD, O_NOFOLLOW etc. +# include <unistd.h> // close, unlinkat #endif namespace std _GLIBCXX_VISIBILITY(default)