Re: [patch] libstdc++/67747 Allocate space for dirent::d_name

[Jonathan Wakely <jwakely at redhat dot com>]

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 <jwakely@redhat.com>
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();
     }