On 02/10/15 14:41 +0200, Florian Weimer wrote:
On 10/02/2015 02:34 PM, Jonathan Wakely wrote:
On 02/10/15 14:16 +0200, Florian Weimer wrote:
On 09/29/2015 01:37 PM, Jonathan Wakely wrote:
POSIX says that dirent::d_name has an unspecified length, so calls to
readdir_r must pass a buffer with enough trailing space for
{NAME_MAX}+1 characters. I wasn't doing that, which works OK on
GNU/Linux and BSD where d_name is a large array, but fails on Solaris
32-bit.
This uses pathconf to get NAME_MAX and allocates a buffer.
This still has a buffer overflow on certain file systems.
You must not use readdir_r, it is deprecated and always insecure. We
should probably mark it as such in the glibc headers.
OK, I'll just use readdir() then. The directory stream is private to
the library type, so the only way to call readdir() concurrently on a
single directory stream is to increment iterators concurrently, which
is undefined anyway.
Right, that's the only case where readdir_r could be theoretically
useful. But it's not a global structure, the callers have to coordinate
anyway, and so you could well use an external lock.
Here's a much simpler patch that just uses readdir, so not need for
pathconf, NAME_MAX and all that jazz.
Tested on GNU/Linux, DragonFly, AIX, committed to trunk.
So that will work as long as readdir() doesn't use a global static
buffer shared between streams, i.e. it meets the POSIX requirement
that "They shall not be affected by a call to readdir() on a different
directory stream." I don't know if mingw meets that, but there is lots
of work needed to make this stuff work in mingw.
If mingw has this flaw, it is worth fixing on its own, and mingw is
sufficiently alive that sticking workarounds into libstdc++ for its bugs
doesn't make sense (IMHO).
FWIW I checked and the readdir in mingw-w64 is OK.
commit 9cef7454bf1a18f21a7a561eef9165149a3330c9
Author: Jonathan Wakely <jwak...@redhat.com>
Date: Fri Oct 2 15:53:09 2015 +0100
PR libstdc++/67747 use readdir instead of readdir_r
PR libstdc++/67747
* src/filesystem/dir.cc (native_readdir): Remove.
(_Dir::advance): Use readdir instead of native_readdir.
(recursive_directory_iterator(const path&, directory_options,
error_code*)): Use swap instead of reset.
diff --git a/libstdc++-v3/src/filesystem/dir.cc b/libstdc++-v3/src/filesystem/dir.cc
index bce751c..33280ec 100644
--- a/libstdc++-v3/src/filesystem/dir.cc
+++ b/libstdc++-v3/src/filesystem/dir.cc
@@ -69,15 +69,17 @@ struct fs::_Dir
namespace
{
template<typename Bitmask>
- inline bool is_set(Bitmask obj, Bitmask bits)
+ inline bool
+ is_set(Bitmask obj, Bitmask bits)
{
return (obj & bits) != Bitmask::none;
}
// Returns {dirp, p} on success, {nullptr, p} on error.
// If an ignored EACCES error occurs returns {}.
- fs::_Dir
- open_dir(const fs::path& p, fs::directory_options options, std::error_code* ec)
+ inline fs::_Dir
+ open_dir(const fs::path& p, fs::directory_options options,
+ std::error_code* ec)
{
if (ec)
ec->clear();
@@ -100,7 +102,7 @@ namespace
}
inline fs::file_type
- get_file_type(const dirent& d __attribute__((__unused__)))
+ get_file_type(const ::dirent& d __attribute__((__unused__)))
{
#ifdef _GLIBCXX_HAVE_STRUCT_DIRENT_D_TYPE
switch (d.d_type)
@@ -128,20 +130,9 @@ namespace
return fs::file_type::none;
#endif
}
-
- int
- native_readdir(DIR* dirp, ::dirent*& entryp)
- {
-#ifdef _GLIBCXX_FILESYSTEM_IS_WINDOWS
- if ((entryp = ::readdir(dirp)))
- return 0;
- return errno;
-#else
- return ::readdir_r(dirp, entryp, &entryp);
-#endif
- }
}
+
// Returns false when the end of the directory entries is reached.
// Reports errors by setting ec or throwing.
bool
@@ -150,9 +141,20 @@ fs::_Dir::advance(error_code* ec, directory_options options)
if (ec)
ec->clear();
- ::dirent ent;
- ::dirent* result = &ent;
- if (int err = native_readdir(dirp, result))
+ int err = std::exchange(errno, 0);
+ const auto entp = readdir(dirp);
+ std::swap(errno, err);
+
+ if (entp)
+ {
+ // skip past dot and dot-dot
+ if (!strcmp(entp->d_name, ".") || !strcmp(entp->d_name, ".."))
+ return advance(ec, options);
+ entry = fs::directory_entry{path / entp->d_name};
+ type = get_file_type(*entp);
+ return true;
+ }
+ else if (err)
{
if (err == EACCES
&& is_set(options, directory_options::skip_permission_denied))
@@ -165,15 +167,6 @@ fs::_Dir::advance(error_code* ec, directory_options options)
ec->assign(err, std::generic_category());
return true;
}
- else if (result != nullptr)
- {
- // skip past dot and dot-dot
- if (!strcmp(ent.d_name, ".") || !strcmp(ent.d_name, ".."))
- return advance(ec, options);
- entry = fs::directory_entry{path / ent.d_name};
- type = get_file_type(ent);
- return true;
- }
else
{
// reached the end
@@ -251,10 +244,10 @@ recursive_directory_iterator(const path& p, directory_options options,
{
if (DIR* dirp = ::opendir(p.c_str()))
{
- _M_dirs = std::make_shared<_Dir_stack>();
- _M_dirs->push(_Dir{ dirp, p });
- if (!_M_dirs->top().advance(ec))
- _M_dirs.reset();
+ auto sp = std::make_shared<_Dir_stack>();
+ sp->push(_Dir{ dirp, p });
+ if (sp->top().advance(ec))
+ _M_dirs.swap(sp);
}
else
{
