On 01/10/15 19:38 +0100, Jonathan Wakely wrote:
On 01/10/15 18:23 +0100, Jonathan Wakely wrote:+ struct op_delete { + void operator()(void* p) const { ::operator delete(p); } + }; + std::unique_ptr<::dirent*, op_delete> ptr;Oops, that should be dirent not dirent* i.e. std::unique_ptr<::dirent, op_delete> ptr; (Found on AIX, where NAME_MAX isn't defined.)
Revised patch fixing that, adjusting native_readdir() to work on AIX where readdir_r returns non-zero at end-of-directory, and following the advice of https://womble.decadent.org.uk/readdir_r-advisory.html by using fpathconf and dirfd. Tested x86_64-dragonfly4.2, powerpc64le-linux and powerpc-aix7.
commit d74845cd00231308a11d41e79309993eb46bb220 Author: Jonathan Wakely <[email protected]> Date: Fri Oct 2 00:20:05 2015 +0100 PR libstdc++/67747 Allocate space for dirent::d_name PR libstdc++/67747 * configure.ac: Check for fpathconf and dirfd. * config.h.in: Regenerate. * configure: Regenerate. * src/filesystem/dir.cc (_Dir::entp): New member. (dirent_size, dirent_buffer): New helpers for readdir results. (native_readdir) [_GLIBCXX_FILESYSTEM_IS_WINDOWS]: Copy to supplied dirent object. Handle end of directory. [_AIX]: Handle non-standard semantics for readdir_r return value. (_Dir::advance): Store readdir result at *entp. (directory_iterator(const path&, directory_options, error_code*)): Allocate a dirent_buffer alongside the _Dir object. (recursive_directory_iterator::_Dir_stack): Add dirent_buffer member, constructor and push() function that sets the element's entp. diff --git a/libstdc++-v3/configure.ac b/libstdc++-v3/configure.ac index 3456348..af9cc7b 100644 --- a/libstdc++-v3/configure.ac +++ b/libstdc++-v3/configure.ac @@ -406,6 +406,7 @@ GLIBCXX_CHECK_GTHREADS AC_CHECK_HEADERS([fcntl.h dirent.h sys/statvfs.h utime.h]) GLIBCXX_ENABLE_FILESYSTEM_TS GLIBCXX_CHECK_FILESYSTEM_DEPS +AC_CHECK_FUNCS(fpathconf dirfd) # Define documentation rules conditionally. diff --git a/libstdc++-v3/src/filesystem/dir.cc b/libstdc++-v3/src/filesystem/dir.cc index bce751c..4cf2bd2 100644 --- a/libstdc++-v3/src/filesystem/dir.cc +++ b/libstdc++-v3/src/filesystem/dir.cc @@ -25,8 +25,12 @@ #include <experimental/filesystem> #include <utility> #include <stack> +#include <stddef.h> #include <string.h> #include <errno.h> +#ifdef _GLIBCXX_HAVE_UNISTD_H +# include <unistd.h> +#endif #ifdef _GLIBCXX_HAVE_DIRENT_H # ifdef _GLIBCXX_HAVE_SYS_TYPES_H # include <sys/types.h> @@ -51,7 +55,7 @@ struct fs::_Dir _Dir(_Dir&& d) : dirp(std::exchange(d.dirp, nullptr)), path(std::move(d.path)), - entry(std::move(d.entry)), type(d.type) + entry(std::move(d.entry)), type(d.type), entp(d.entp) { } _Dir& operator=(_Dir&&) = delete; @@ -64,20 +68,72 @@ struct fs::_Dir fs::path path; directory_entry entry; file_type type = file_type::none; + ::dirent* entp = nullptr; }; namespace { +#ifdef _GLIBCXX_FILESYSTEM_IS_WINDOWS +# undef NAME_MAX +# define NAME_MAX 260 +#endif + + // Size needed for struct dirent with {NAME_MAX} + 1 chars. + static constexpr size_t + dirent_size(size_t name_max) + { + if (name_max < 255) + name_max = 255; + return offsetof(::dirent, d_name) + name_max + 1; + } + + // Manage a buffer large enough for struct dirent with {NAME_MAX} + 1 chars. + struct dirent_buffer + { +#ifdef NAME_MAX + dirent_buffer(const fs::_Dir&) { } + + ::dirent* get() { return reinterpret_cast<::dirent*>(&ent); } + + std::aligned_union<dirent_size(NAME_MAX), ::dirent>::type ent; +#else + + dirent_buffer(const fs::_Dir& d __attribute__((__unused__))) + { + long name_max = 255; // An informed guess. +#ifdef _GLIBCXX_HAVE_UNISTD_H +# if _GLIBCXX_HAVE_FPATHCONF && _GLIBCXX_HAVE_DIRFD + long pc_name_max = ::fpathconf(::dirfd(d.dirp), _PC_NAME_MAX); +# else + long pc_name_max = ::pathconf(d.path.c_str(), _PC_NAME_MAX); +# endif + if (pc_name_max != -1) + name_max = pc_name_max; +#endif + ptr.reset(static_cast<::dirent*>(::operator new(dirent_size(name_max)))); + } + + ::dirent* get() const { return ptr.get(); } + + struct op_delete { + void operator()(void* p) const { ::operator delete(p); } + }; + std::unique_ptr<::dirent, op_delete> ptr; +#endif + }; + 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 +156,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) @@ -129,19 +185,42 @@ namespace #endif } - int + inline int native_readdir(DIR* dirp, ::dirent*& entryp) { #ifdef _GLIBCXX_FILESYSTEM_IS_WINDOWS - if ((entryp = ::readdir(dirp))) - return 0; + const int saved_errno = errno; + errno = 0; + if (auto entp = ::readdir(dirp)) + { + errno = saved_errno; + memcpy(entryp, entp, dirent_size(strlen(entp->d_name))); + return 0; + } + else if (errno == 0) // End of directory reached. + { + errno = saved_errno; + entryp = nullptr; + return 0; + } return errno; #else - return ::readdir_r(dirp, entryp, &entryp); + const int err = ::readdir_r(dirp, entryp, &entryp); +#ifdef _AIX + if (err == 9) + { + if (entryp == nullptr) + return 0; + else + return errno; + } +#endif + return err; #endif } } + // Returns false when the end of the directory entries is reached. // Reports errors by setting ec or throwing. bool @@ -150,8 +229,7 @@ fs::_Dir::advance(error_code* ec, directory_options options) if (ec) ec->clear(); - ::dirent ent; - ::dirent* result = &ent; + auto result = entp; if (int err = native_readdir(dirp, result)) { if (err == EACCES @@ -168,10 +246,10 @@ fs::_Dir::advance(error_code* ec, directory_options options) else if (result != nullptr) { // skip past dot and dot-dot - if (!strcmp(ent.d_name, ".") || !strcmp(ent.d_name, "..")) + if (!strcmp(entp->d_name, ".") || !strcmp(entp->d_name, "..")) return advance(ec, options); - entry = fs::directory_entry{path / ent.d_name}; - type = get_file_type(ent); + entry = fs::directory_entry{path / entp->d_name}; + type = get_file_type(*entp); return true; } else @@ -190,9 +268,17 @@ directory_iterator(const path& p, directory_options options, error_code* ec) if (dir.dirp) { - auto sp = std::make_shared<fs::_Dir>(std::move(dir)); - if (sp->advance(ec, options)) - _M_dir.swap(sp); + // A _Dir with an associated dirent_buffer. + struct Dir_with_buffer : _Dir + { + Dir_with_buffer(_Dir&& d) : _Dir(std::move(d)), buf(*this) + { this->entp = buf.get(); } + + dirent_buffer buf; + }; + _M_dir = std::make_shared<Dir_with_buffer>(std::move(dir)); + if (!_M_dir->advance(ec, options)) + _M_dir.reset(); } else if (!dir.path.empty()) { @@ -241,7 +327,17 @@ using Dir_iter_pair = std::pair<fs::_Dir, fs::directory_iterator>; struct fs::recursive_directory_iterator::_Dir_stack : std::stack<_Dir> { + _Dir_stack(_Dir&& d) : buf(d) { push(std::move(d)); } + + void push(_Dir&& d) + { + d.entp = buf.get(); + stack::push(std::move(d)); + } + void clear() { c.clear(); } + + dirent_buffer buf; }; fs::recursive_directory_iterator:: @@ -251,8 +347,7 @@ 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 }); + _M_dirs = std::make_shared<_Dir_stack>(_Dir{ dirp, p }); if (!_M_dirs->top().advance(ec)) _M_dirs.reset(); }
