* [PATCH] libstdc++: Decouple HAVE_FCNTL_H from HAVE_DIRENT_H check
@ 2022-02-07 20:10 Dimitar Dimitrov
2022-02-07 21:01 ` Jonathan Wakely
0 siblings, 1 reply; 6+ messages in thread
From: Dimitar Dimitrov @ 2022-02-07 20:10 UTC (permalink / raw)
To: libstdc++, gcc-patches; +Cc: Jonathan Wakely, Dimitar Dimitrov
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(-)
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] libstdc++: Decouple HAVE_FCNTL_H from HAVE_DIRENT_H check
2022-02-07 20:10 [PATCH] libstdc++: Decouple HAVE_FCNTL_H from HAVE_DIRENT_H check Dimitar Dimitrov
@ 2022-02-07 21:01 ` Jonathan Wakely
2022-02-07 21:05 ` Jonathan Wakely
0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Wakely @ 2022-02-07 21:01 UTC (permalink / raw)
To: Dimitar Dimitrov; +Cc: libstdc++, gcc-patches, Jonathan Wakely
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
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] libstdc++: Decouple HAVE_FCNTL_H from HAVE_DIRENT_H check
2022-02-07 21:01 ` Jonathan Wakely
@ 2022-02-07 21:05 ` Jonathan Wakely
2022-02-08 21:02 ` Dimitar Dimitrov
0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Wakely @ 2022-02-07 21:05 UTC (permalink / raw)
To: Dimitar Dimitrov; +Cc: libstdc++, gcc-patches, Jonathan Wakely
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.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] libstdc++: Decouple HAVE_FCNTL_H from HAVE_DIRENT_H check
2022-02-07 21:05 ` Jonathan Wakely
@ 2022-02-08 21:02 ` Dimitar Dimitrov
2022-02-08 21:18 ` Jonathan Wakely
0 siblings, 1 reply; 6+ messages in thread
From: Dimitar Dimitrov @ 2022-02-08 21:02 UTC (permalink / raw)
To: Jonathan Wakely; +Cc: libstdc++, gcc-patches, Jonathan Wakely
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] libstdc++: Decouple HAVE_FCNTL_H from HAVE_DIRENT_H check
2022-02-08 21:02 ` Dimitar Dimitrov
@ 2022-02-08 21:18 ` Jonathan Wakely
2022-02-10 13:03 ` [committed] " Jonathan Wakely
0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Wakely @ 2022-02-08 21:18 UTC (permalink / raw)
To: Dimitar Dimitrov; +Cc: libstdc++, gcc-patches, Jonathan Wakely
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.
^ permalink raw reply [flat|nested] 6+ messages in thread* [committed] libstdc++: Decouple HAVE_FCNTL_H from HAVE_DIRENT_H check
2022-02-08 21:18 ` Jonathan Wakely
@ 2022-02-10 13:03 ` Jonathan Wakely
0 siblings, 0 replies; 6+ messages in thread
From: Jonathan Wakely @ 2022-02-10 13:03 UTC (permalink / raw)
To: Dimitar Dimitrov; +Cc: libstdc++, gcc-patches, Jonathan Wakely
[-- Attachment #1: Type: text/plain, Size: 2401 bytes --]
On Tue, 8 Feb 2022 at 21:18, Jonathan Wakely wrote:
>
>
> 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.
>
>
Sorry for the delay, that's pushed to trunk now.
[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 1221 bytes --]
commit 3d5f4f76e6db0895181ebca538748379bfe6058f
Author: Jonathan Wakely <jwakely@redhat.com>
Date: Tue Feb 8 21:05:30 2022
libstdc++: Fix directory iterator build for newlib
When building for newlib HAVE_OPENAT and HAVE_UNLINKAT are (sometimes?)
defined, but <fcntl.h> is only included when HAVE_DIRENT_H is defined.
Since directory iterators are completely useless without <dirent.h>,
just override the HAVE_OPENAT and HAVE_UNLINKAT detection when we don't
have <dirent.h>.
libstdc++-v3/ChangeLog:
* src/filesystem/dir-common.h (_GLIBCXX_HAVE_DIRFD): Undefine
when <dirent.h> is not available.
(_GLIBCXX_HAVE_UNLINKAT): Likewise.
diff --git a/libstdc++-v3/src/filesystem/dir-common.h b/libstdc++-v3/src/filesystem/dir-common.h
index 511b988f1c7..365fd527f4d 100644
--- 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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-02-10 13:03 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-07 20:10 [PATCH] libstdc++: Decouple HAVE_FCNTL_H from HAVE_DIRENT_H check Dimitar Dimitrov
2022-02-07 21:01 ` Jonathan Wakely
2022-02-07 21:05 ` Jonathan Wakely
2022-02-08 21:02 ` Dimitar Dimitrov
2022-02-08 21:18 ` Jonathan Wakely
2022-02-10 13:03 ` [committed] " Jonathan Wakely
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).