Re: [PATCH] libstdc++: Decouple HAVE_FCNTL_H from HAVE_DIRENT_H check
On Tue, 8 Feb 2022 at 21:02, Dimitar Dimitrov wrote: > On Mon, Feb 07, 2022 at 09:05:45PM +, Jonathan Wakely wrote: > > On Mon, 7 Feb 2022 at 21:01, Jonathan Wakely > wrote: > > > > > > > > > > > On Mon, 7 Feb 2022 at 20:12, Dimitar Dimitrov > 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 : > > >> > > >> > 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 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 include with the correct check. > > >> > > > > > > But there's no point doing anything in that file if we don't have > > > , 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.
Re: [PATCH] libstdc++: Decouple HAVE_FCNTL_H from HAVE_DIRENT_H check
On Mon, Feb 07, 2022 at 09:05:45PM +, Jonathan Wakely wrote: > On Mon, 7 Feb 2022 at 21:01, Jonathan Wakely wrote: > > > > > > > On Mon, 7 Feb 2022 at 20:12, Dimitar Dimitrov 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 : > >> > >> 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 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 include with the correct check. > >> > > > > But there's no point doing anything in that file if we don't have > > , 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
Re: [PATCH] libstdc++: Decouple HAVE_FCNTL_H from HAVE_DIRENT_H check
On Mon, 7 Feb 2022 at 21:01, Jonathan Wakely wrote: > > > On Mon, 7 Feb 2022 at 20:12, Dimitar Dimitrov 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 : >> >> 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 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 include with the correct check. >> > > But there's no point doing anything in that file if we don't have > , 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.
Re: [PATCH] libstdc++: Decouple HAVE_FCNTL_H from HAVE_DIRENT_H check
On Mon, 7 Feb 2022 at 20:12, Dimitar Dimitrov 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 : > > 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 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 include with the correct check. > But there's no point doing anything in that file if we don't have , 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 > --- > 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 > # endif > # include // opendir, readdir, fdopendir, dirfd > -# ifdef _GLIBCXX_HAVE_FCNTL_H > -# include // open, openat, fcntl, AT_FDCWD, O_NOFOLLOW etc. > -# include // close, unlinkat > -# endif > +#endif > + > +#ifdef _GLIBCXX_HAVE_FCNTL_H > +# include // open, openat, fcntl, AT_FDCWD, O_NOFOLLOW etc. > +# include // close, unlinkat > #endif > > namespace std _GLIBCXX_VISIBILITY(default) > -- > 2.34.1 > >