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 > >
