public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Five patches for std::experimental::filesystem
@ 2016-10-24 16:50 Jonathan Wakely
  2016-10-25 15:32 ` Jonathan Wakely
  2016-10-26 13:34 ` Jonathan Wakely
  0 siblings, 2 replies; 8+ messages in thread
From: Jonathan Wakely @ 2016-10-24 16:50 UTC (permalink / raw)
  To: libstdc++, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 2281 bytes --]

These implement DR resolutions and fix bugs.

    Fix error handling in filesystem::is_empty
    
        * src/filesystem/ops.cc (is_empty): Fix error handling.
        * testsuite/experimental/filesystem/operations/is_empty.cc: New test.

    PR71337 fix filesystem::temp_directory_path error handling
    
        PR libstdc++/71337
        * src/filesystem/ops.cc (temp_directory_path): Pass error_code
        argument to other filesystem operations.
        * testsuite/experimental/filesystem/operations/temp_directory_path.cc:
        Add testcase for inaccessible directory.

    Make directory iterators become end iterator on error
    
        * src/filesystem/dir.cc (open_dir): Return same value for errors
        whether ignored or not.
        (_Dir::advance(error_code*, directory_options)): Return false on
        error.
        (directory_iterator(const path&, directory_options, error_code*)):
        Create end iterator on error (LWG 2723).
        (recursive_directory_iterator(const path&, directory_options,
        error_code*)): Likewise.
        * testsuite/experimental/filesystem/iterators/directory_iterator.cc:
        Update expected behaviour on error.
        * testsuite/experimental/filesystem/iterators/
        recursive_directory_iterator.cc: Likewise.

    Do not retry failed close(3) in filesystem::copy
    
        * src/filesystem/ops.cc (close_fd): Remove.
        (do_copy_file): Just use close(3) instead of close_fd, to prevent
        retrying on error.

    Implement DR resolutions for filesystem::copy
    
        * src/filesystem/ops.cc (do_copy_file): Return an error if either
        source or destination is not a regular file.
        (copy): Update comment to refer to LWG 2681. Implement 2682 and 2683
        resolutions.
        (read_symlink): Add missing ec.clear().
        * testsuite/experimental/filesystem/operations/copy.cc: Update
        expected behaviour for copying directories with create_symlinks.
        Verify that error_code arguments are cleared if there's no error.
        * testsuite/experimental/filesystem/operations/read_symlink.cc:
        * New.

Tested x86_64-linux, commiitted to trunk. I'm going to do some mass
backports for all filesystem fixes to the branches this week as well.


[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 17856 bytes --]

commit 363cdc78be5edb191411e57206d94e18e42ec259
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Mon Oct 24 15:50:51 2016 +0100

    Fix error handling in filesystem::is_empty
    
    	* src/filesystem/ops.cc (is_empty): Fix error handling.
    	* testsuite/experimental/filesystem/operations/is_empty.cc: New test.

diff --git a/libstdc++-v3/src/filesystem/ops.cc b/libstdc++-v3/src/filesystem/ops.cc
index 90c225b..d9a12df 100644
--- a/libstdc++-v3/src/filesystem/ops.cc
+++ b/libstdc++-v3/src/filesystem/ops.cc
@@ -1022,20 +1022,24 @@ fs::hard_link_count(const path& p, error_code& ec) noexcept
 bool
 fs::is_empty(const path& p)
 {
-  return fs::is_directory(status(p))
-    ? fs::directory_iterator(p) == fs::directory_iterator()
-    : fs::file_size(p) == 0;
+  error_code ec;
+  bool e = is_empty(p, ec);
+  if (ec)
+    _GLIBCXX_THROW_OR_ABORT(filesystem_error("cannot check is file is empty",
+					     p, ec));
+  return e;
 }
 
 bool
 fs::is_empty(const path& p, error_code& ec) noexcept
 {
   auto s = status(p, ec);
-  if (ec.value())
+  if (ec)
     return false;
-  return fs::is_directory(s)
+  bool empty = fs::is_directory(s)
     ? fs::directory_iterator(p, ec) == fs::directory_iterator()
     : fs::file_size(p, ec) == 0;
+  return ec ? false : empty;
 }
 
 fs::file_time_type

commit 4d32e1be1f623b42743092aefb7388019bea0c7f
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Mon Oct 24 15:27:00 2016 +0100

    PR71337 fix filesystem::temp_directory_path error handling
    
    	PR libstdc++/71337
    	* src/filesystem/ops.cc (temp_directory_path): Pass error_code
    	argument to other filesystem operations.
    	* testsuite/experimental/filesystem/operations/temp_directory_path.cc:
    	Add testcase for inaccessible directory.

diff --git a/libstdc++-v3/src/filesystem/ops.cc b/libstdc++-v3/src/filesystem/ops.cc
index f8ba74e..90c225b 100644
--- a/libstdc++-v3/src/filesystem/ops.cc
+++ b/libstdc++-v3/src/filesystem/ops.cc
@@ -1428,12 +1428,17 @@ fs::path fs::temp_directory_path(error_code& ec)
   for (auto e = env; tmpdir == nullptr && *e != nullptr; ++e)
     tmpdir = ::getenv(*e);
   path p = tmpdir ? tmpdir : "/tmp";
-  if (exists(p) && is_directory(p))
+  auto st = status(p, ec);
+  if (!ec)
     {
-      ec.clear();
-      return p;
+      if (is_directory(st))
+	{
+	  ec.clear();
+	  return p;
+	}
+      else
+	ec = std::make_error_code(std::errc::not_a_directory);
     }
-  ec = std::make_error_code(std::errc::not_a_directory);
   return {};
 #endif
 }
diff --git a/libstdc++-v3/testsuite/experimental/filesystem/operations/temp_directory_path.cc b/libstdc++-v3/testsuite/experimental/filesystem/operations/temp_directory_path.cc
index 6e202c9..7f7e9fd 100644
--- a/libstdc++-v3/testsuite/experimental/filesystem/operations/temp_directory_path.cc
+++ b/libstdc++-v3/testsuite/experimental/filesystem/operations/temp_directory_path.cc
@@ -45,6 +45,7 @@ test01()
 
   std::error_code ec;
   fs::path p1 = fs::temp_directory_path(ec);
+  VERIFY( !ec );
   VERIFY( exists(p1) );
 
   fs::path p2 = fs::temp_directory_path();
@@ -62,6 +63,7 @@ test02()
   std::error_code ec;
   fs::path p = fs::temp_directory_path(ec);
   VERIFY( ec );
+  VERIFY( p == fs::path() );
 
   std::error_code ec2;
   try {
@@ -72,10 +74,54 @@ test02()
   VERIFY( ec2 == ec );
 }
 
+void
+test03()
+{
+  auto p = __gnu_test::nonexistent_path();
+  create_directories(p/"tmp");
+  permissions(p, fs::perms::none);
+  setenv("TMPDIR", (p/"tmp").c_str(), 1);
+  std::error_code ec;
+  auto r = fs::temp_directory_path(ec); // libstdc++/PR71337
+  VERIFY( ec == std::make_error_code(std::errc::permission_denied) );
+  VERIFY( r == fs::path() );
+
+  std::error_code ec2;
+  try {
+    fs::temp_directory_path();
+  } catch (const fs::filesystem_error& e) {
+    ec2 = e.code();
+  }
+  VERIFY( ec2 == ec );
+
+  permissions(p, fs::perms::owner_all, ec);
+  remove_all(p, ec);
+}
+
+void
+test04()
+{
+  __gnu_test::scoped_file f;
+  setenv("TMPDIR", f.path.c_str(), 1);
+  std::error_code ec;
+  auto r = fs::temp_directory_path(ec);
+  VERIFY( ec == std::make_error_code(std::errc::not_a_directory) );
+  VERIFY( r == fs::path() );
+
+  std::error_code ec2;
+  try {
+    fs::temp_directory_path();
+  } catch (const fs::filesystem_error& e) {
+    ec2 = e.code();
+  }
+  VERIFY( ec2 == ec );
+}
 
 int
 main()
 {
   test01();
   test02();
+  test03();
+  test04();
 }

commit 015912fca75b8747123efbf5419e3e8be9b0a5b9
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Mon Oct 24 14:50:01 2016 +0100

    Make directory iterators become end iterator on error
    
    	* src/filesystem/dir.cc (open_dir): Return same value for errors
    	whether ignored or not.
    	(_Dir::advance(error_code*, directory_options)): Return false on
    	error.
    	(directory_iterator(const path&, directory_options, error_code*)):
    	Create end iterator on error (LWG 2723).
    	(recursive_directory_iterator(const path&, directory_options,
    	error_code*)): Likewise.
    	* testsuite/experimental/filesystem/iterators/directory_iterator.cc:
    	Update expected behaviour on error.
    	* testsuite/experimental/filesystem/iterators/
    	recursive_directory_iterator.cc: Likewise.

diff --git a/libstdc++-v3/src/filesystem/dir.cc b/libstdc++-v3/src/filesystem/dir.cc
index 6ff12d0..4640d75 100644
--- a/libstdc++-v3/src/filesystem/dir.cc
+++ b/libstdc++-v3/src/filesystem/dir.cc
@@ -79,8 +79,7 @@ namespace
       return (obj & bits) != Bitmask::none;
     }
 
-  // Returns {dirp, p} on success, {nullptr, p} on error.
-  // If an ignored EACCES error occurs returns {}.
+  // Returns {dirp, p} on success, {} on error (whether ignored or not).
   inline fs::_Dir
   open_dir(const fs::path& p, fs::directory_options options,
 	   std::error_code* ec)
@@ -102,7 +101,7 @@ namespace
             std::error_code(err, std::generic_category())));
 
     ec->assign(err, std::generic_category());
-    return {nullptr, p};
+    return {};
   }
 
   inline fs::file_type
@@ -169,7 +168,7 @@ fs::_Dir::advance(error_code* ec, directory_options options)
 	      "directory iterator cannot advance",
 	      std::error_code(err, std::generic_category())));
       ec->assign(err, std::generic_category());
-      return true;
+      return false;
     }
   else
     {
@@ -191,12 +190,6 @@ directory_iterator(const path& p, directory_options options, error_code* ec)
       if (sp->advance(ec, options))
 	_M_dir.swap(sp);
     }
-  else if (!dir.path.empty())
-    {
-      // An error occurred, we need a non-empty shared_ptr so that *this will
-      // not compare equal to the end iterator.
-      _M_dir.reset(static_cast<fs::_Dir*>(nullptr));
-    }
 }
 
 const fs::directory_entry&
@@ -270,10 +263,6 @@ recursive_directory_iterator(const path& p, directory_options options,
 	      std::error_code(err, std::generic_category())));
 
       ec->assign(err, std::generic_category());
-
-      // An error occurred, we need a non-empty shared_ptr so that *this will
-      // not compare equal to the end iterator.
-      _M_dirs.reset(static_cast<_Dir_stack*>(nullptr));
     }
 }
 
diff --git a/libstdc++-v3/testsuite/experimental/filesystem/iterators/directory_iterator.cc b/libstdc++-v3/testsuite/experimental/filesystem/iterators/directory_iterator.cc
index 5c80fb7..5788700 100644
--- a/libstdc++-v3/testsuite/experimental/filesystem/iterators/directory_iterator.cc
+++ b/libstdc++-v3/testsuite/experimental/filesystem/iterators/directory_iterator.cc
@@ -34,7 +34,7 @@ test01()
   const auto p = __gnu_test::nonexistent_path();
   fs::directory_iterator iter(p, ec);
   VERIFY( ec );
-  VERIFY( iter != fs::directory_iterator() );
+  VERIFY( iter == fs::directory_iterator() );
 
   // Test empty directory.
   create_directory(p, fs::current_path(), ec);
@@ -58,7 +58,7 @@ test01()
   VERIFY( !ec );
   iter = fs::directory_iterator(p, ec);
   VERIFY( ec );
-  VERIFY( iter != fs::directory_iterator() );
+  VERIFY( iter == fs::directory_iterator() );
 
   // Test inaccessible directory, skipping permission denied.
   const auto opts = fs::directory_options::skip_permission_denied;
diff --git a/libstdc++-v3/testsuite/experimental/filesystem/iterators/recursive_directory_iterator.cc b/libstdc++-v3/testsuite/experimental/filesystem/iterators/recursive_directory_iterator.cc
index 37b2606..b41c394 100644
--- a/libstdc++-v3/testsuite/experimental/filesystem/iterators/recursive_directory_iterator.cc
+++ b/libstdc++-v3/testsuite/experimental/filesystem/iterators/recursive_directory_iterator.cc
@@ -34,7 +34,7 @@ test01()
   const auto p = __gnu_test::nonexistent_path();
   fs::recursive_directory_iterator iter(p, ec);
   VERIFY( ec );
-  VERIFY( iter != fs::recursive_directory_iterator() );
+  VERIFY( iter == fs::recursive_directory_iterator() );
 
   // Test empty directory.
   create_directory(p, fs::current_path(), ec);
@@ -60,7 +60,7 @@ test01()
   VERIFY( !ec );
   iter = fs::recursive_directory_iterator(p, ec);
   VERIFY( ec );
-  VERIFY( iter != fs::recursive_directory_iterator() );
+  VERIFY( iter == fs::recursive_directory_iterator() );
 
   // Test inaccessible directory, skipping permission denied.
   const auto opts = fs::directory_options::skip_permission_denied;

commit ea465ff7461e814a8e2f83007653428a403eadb0
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Mon Oct 24 14:39:24 2016 +0100

    Do not retry failed close(3) in filesystem::copy
    
    	* src/filesystem/ops.cc (close_fd): Remove.
    	(do_copy_file): Just use close(3) instead of close_fd, to prevent
    	retrying on error.

diff --git a/libstdc++-v3/src/filesystem/ops.cc b/libstdc++-v3/src/filesystem/ops.cc
index 6f76053..f8ba74e 100644
--- a/libstdc++-v3/src/filesystem/ops.cc
+++ b/libstdc++-v3/src/filesystem/ops.cc
@@ -308,17 +308,6 @@ namespace
     return fs::file_time_type{seconds{s} + ns};
   }
 
-  // Returns true if the file descriptor was successfully closed,
-  // otherwise returns false and the reason will be in errno.
-  inline bool
-  close_fd(int fd)
-  {
-    while (::close(fd))
-      if (errno != EINTR)
-	return false;
-    return true;
-  }
-
   bool
   do_copy_file(const fs::path& from, const fs::path& to,
 	       fs::copy_options option,
@@ -405,8 +394,8 @@ namespace
       }
 
     struct CloseFD {
-      ~CloseFD() { if (fd != -1) close_fd(fd); }
-      bool close() { return close_fd(std::exchange(fd, -1)); }
+      ~CloseFD() { if (fd != -1) ::close(fd); }
+      bool close() { return ::close(std::exchange(fd, -1)) == 0; }
       int fd;
     };
 

commit 68eea18d5d6aa635f8d1b53e780e7fe703cd7610
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Mon Oct 24 14:33:27 2016 +0100

    Implement DR resolutions for filesystem::copy
    
    	* src/filesystem/ops.cc (do_copy_file): Return an error if either
    	source or destination is not a regular file.
    	(copy): Update comment to refer to LWG 2681. Implement 2682 and 2683
    	resolutions.
    	(read_symlink): Add missing ec.clear().
    	* testsuite/experimental/filesystem/operations/copy.cc: Update
    	expected behaviour for copying directories with create_symlinks.
    	Verify that error_code arguments are cleared if there's no error.
    	* testsuite/experimental/filesystem/operations/read_symlink.cc: New.

diff --git a/libstdc++-v3/src/filesystem/ops.cc b/libstdc++-v3/src/filesystem/ops.cc
index 2286e22..6f76053 100644
--- a/libstdc++-v3/src/filesystem/ops.cc
+++ b/libstdc++-v3/src/filesystem/ops.cc
@@ -361,6 +361,11 @@ namespace
 	  from_st = &st2;
       }
     f = make_file_status(*from_st);
+    if (!is_regular_file(f))
+      {
+	ec = std::make_error_code(std::errc::not_supported);
+	return false;
+      }
 
     using opts = fs::copy_options;
 
@@ -392,6 +397,11 @@ namespace
 	    ec = std::make_error_code(std::errc::file_exists);
 	    return false;
 	  }
+	else if (!is_regular_file(t))
+	  {
+	    ec = std::make_error_code(std::errc::not_supported);
+	    return false;
+	  }
       }
 
     struct CloseFD {
@@ -489,7 +499,8 @@ fs::copy(const path& from, const path& to, copy_options options,
 
   file_status f, t;
   stat_type from_st, to_st;
-  // N4099 doesn't check copy_symlinks here, but I think that's a defect.
+  // _GLIBCXX_RESOLVE_LIB_DEFECTS
+  // 2681. filesystem::copy() cannot copy symlinks
   if (use_lstat || copy_symlinks
       ? ::lstat(from.c_str(), &from_st)
       : ::stat(from.c_str(), &from_st))
@@ -556,6 +567,10 @@ fs::copy(const path& from, const path& to, copy_options options,
 	  do_copy_file(from, to, options, &from_st, ptr,  ec);
 	}
     }
+  // _GLIBCXX_RESOLVE_LIB_DEFECTS
+  // 2682. filesystem::copy() won't create a symlink to a directory
+  else if (is_directory(f) && create_symlinks)
+    ec = std::make_error_code(errc::is_a_directory);
   else if (is_directory(f) && (is_set(options, copy_options::recursive)
 			       || options == copy_options::none))
     {
@@ -568,7 +583,10 @@ fs::copy(const path& from, const path& to, copy_options options,
       for (const directory_entry& x : directory_iterator(from))
 	copy(x.path(), to/x.path().filename(), options, ec);
     }
-  // "Otherwise no effects." (should ec.clear() be called?)
+  // _GLIBCXX_RESOLVE_LIB_DEFECTS
+  // 2683. filesystem::copy() says "no effects"
+  else
+    ec.clear();
 }
 
 bool
@@ -1168,6 +1186,7 @@ fs::path fs::read_symlink(const path& p, error_code& ec)
       ec.assign(errno, std::generic_category());
       return {};
     }
+  ec.clear();
   return path{buf.data(), buf.data()+len};
 #else
   ec = std::make_error_code(std::errc::not_supported);
diff --git a/libstdc++-v3/testsuite/experimental/filesystem/operations/copy.cc b/libstdc++-v3/testsuite/experimental/filesystem/operations/copy.cc
index 0d112a2..2cfb1c1 100644
--- a/libstdc++-v3/testsuite/experimental/filesystem/operations/copy.cc
+++ b/libstdc++-v3/testsuite/experimental/filesystem/operations/copy.cc
@@ -22,7 +22,6 @@
 // 15.3 Copy [fs.op.copy]
 
 #include <experimental/filesystem>
-#include <fstream>
 #include <testsuite_fs.h>
 #include <testsuite_hooks.h>
 
@@ -43,14 +42,25 @@ test01()
   fs::copy(".", ".", fs::copy_options::none, ec);
   VERIFY( ec );
 
-  std::ofstream{p.native()};
+  __gnu_test::scoped_file f(p);
   VERIFY( fs::is_directory(".") );
   VERIFY( fs::is_regular_file(p) );
   ec.clear();
   fs::copy(".", p, fs::copy_options::none, ec);
   VERIFY( ec );
 
-  remove(p, ec);
+  auto to = __gnu_test::nonexistent_path();
+  ec.clear();
+  auto opts = fs::copy_options::create_symlinks;
+  fs::copy("/", to, opts, ec);
+  VERIFY( ec == std::make_error_code(std::errc::is_a_directory) );
+  VERIFY( !exists(to) );
+
+  ec.clear();
+  opts != fs::copy_options::recursive;
+  fs::copy("/", to, opts, ec);
+  VERIFY( ec == std::make_error_code(std::errc::is_a_directory) );
+  VERIFY( !exists(to) );
 }
 
 // Test is_symlink(f) case.
@@ -59,29 +69,35 @@ test02()
 {
   auto from = __gnu_test::nonexistent_path();
   auto to = __gnu_test::nonexistent_path();
-  std::error_code ec;
+  std::error_code ec, bad = std::make_error_code(std::errc::invalid_argument);
 
+  ec = bad;
   fs::create_symlink(".", from, ec);
   VERIFY( !ec );
   VERIFY( fs::exists(from) );
 
+  ec = bad;
   fs::copy(from, to, fs::copy_options::skip_symlinks, ec);
   VERIFY( !ec );
   VERIFY( !fs::exists(to) );
 
+  ec = bad;
   fs::copy(from, to, fs::copy_options::skip_symlinks, ec);
   VERIFY( !ec );
   VERIFY( !fs::exists(to) );
 
+  ec = bad;
   fs::copy(from, to,
            fs::copy_options::skip_symlinks|fs::copy_options::copy_symlinks,
            ec);
   VERIFY( !ec );
   VERIFY( !fs::exists(to) );
 
+  ec = bad;
   fs::copy(from, to, fs::copy_options::copy_symlinks, ec);
   VERIFY( !ec );
   VERIFY( fs::exists(to) );
+  VERIFY( is_symlink(to) );
 
   fs::copy(from, to, fs::copy_options::copy_symlinks, ec);
   VERIFY( ec );
@@ -129,10 +145,10 @@ void
 test05()
 {
   auto to = __gnu_test::nonexistent_path();
-  std::error_code ec;
+  std::error_code ec = std::make_error_code(std::errc::invalid_argument);
 
-  fs::copy("/", to, fs::copy_options::create_symlinks, ec);
-  VERIFY( !ec );
+  fs::copy("/", to, fs::copy_options::copy_symlinks, ec);
+  VERIFY( !ec );  // Previous value should be cleared (LWG 2683)
 }
 
 int
diff --git a/libstdc++-v3/testsuite/experimental/filesystem/operations/read_symlink.cc b/libstdc++-v3/testsuite/experimental/filesystem/operations/read_symlink.cc
new file mode 100644
index 0000000..c4137bd
--- /dev/null
+++ b/libstdc++-v3/testsuite/experimental/filesystem/operations/read_symlink.cc
@@ -0,0 +1,49 @@
+// Copyright (C) 2016 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-lstdc++fs" }
+// { dg-do run { target c++11 } }
+// { dg-require-filesystem-ts "" }
+
+#include <experimental/filesystem>
+#include <testsuite_hooks.h>
+#include <testsuite_fs.h>
+
+namespace fs = std::experimental::filesystem;
+
+void
+test01()
+{
+  auto p = __gnu_test::nonexistent_path();
+  std::error_code ec;
+
+  read_symlink(p, ec);
+  VERIFY( ec );
+
+  fs::path tgt = ".";
+  create_symlink(tgt, p);
+
+  auto result = read_symlink(p, ec);
+  VERIFY( !ec );
+  VERIFY( result == tgt );
+}
+
+int
+main()
+{
+  test01();
+}

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Five patches for std::experimental::filesystem
  2016-10-24 16:50 [PATCH] Five patches for std::experimental::filesystem Jonathan Wakely
@ 2016-10-25 15:32 ` Jonathan Wakely
  2016-10-26  7:24   ` Christophe Lyon
  2016-10-26 13:34 ` Jonathan Wakely
  1 sibling, 1 reply; 8+ messages in thread
From: Jonathan Wakely @ 2016-10-25 15:32 UTC (permalink / raw)
  To: libstdc++, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 1395 bytes --]

Two more fixes for the filesystem TS, and improved tests.

   Handle negative times in filesystem::last_write_time
    
        * src/filesystem/ops.cc
        (last_write_time(const path&, file_time_type, error_code&)): Handle
        negative times correctly.
        * testsuite/experimental/filesystem/operations/last_write_time.cc:
        Test writing file times.

    Fix error handling in copy_file and equivalent
    
        * src/filesystem/ops.cc (do_copy_file): Report an error if source or
        destination is not a regular file (LWG 2712).
        (equivalent): Fix error handling and result when only one file exists.
        * testsuite/experimental/filesystem/operations/copy.cc: Remove files
        created by tests. Test copying directories.
        * testsuite/experimental/filesystem/operations/copy_file.cc: Remove
        files created by tests.
        * testsuite/experimental/filesystem/operations/equivalent.cc: New.
        * testsuite/experimental/filesystem/operations/is_empty.cc: New.
        * testsuite/experimental/filesystem/operations/read_symlink.cc: Remove
        file created by test.
        * testsuite/experimental/filesystem/operations/remove_all.cc: New.
        * testsuite/util/testsuite_fs.h (~scoped_file): Only try to remove
        file if path is non-empty, to support removal by other means.

Tested x86_64-linux, committed to trunk.



[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 17497 bytes --]

commit c58bcab6fc126fa2cf4d3935e21186fa873be980
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Tue Oct 25 00:20:59 2016 +0100

    Handle negative times in filesystem::last_write_time
    
    	* src/filesystem/ops.cc
    	(last_write_time(const path&, file_time_type, error_code&)): Handle
    	negative times correctly.
    	* testsuite/experimental/filesystem/operations/last_write_time.cc:
    	Test writing file times.

diff --git a/libstdc++-v3/src/filesystem/ops.cc b/libstdc++-v3/src/filesystem/ops.cc
index fcf747e..32c9c5e 100644
--- a/libstdc++-v3/src/filesystem/ops.cc
+++ b/libstdc++-v3/src/filesystem/ops.cc
@@ -1100,6 +1100,11 @@ fs::last_write_time(const path& p __attribute__((__unused__)),
   auto s = chrono::duration_cast<chrono::seconds>(d);
 #if _GLIBCXX_USE_UTIMENSAT
   auto ns = chrono::duration_cast<chrono::nanoseconds>(d - s);
+  if (ns < ns.zero()) // tv_nsec must be non-negative and less than 10e9.
+    {
+      --s;
+      ns += chrono::seconds(1);
+    }
   struct ::timespec ts[2];
   ts[0].tv_sec = 0;
   ts[0].tv_nsec = UTIME_OMIT;
diff --git a/libstdc++-v3/testsuite/experimental/filesystem/operations/last_write_time.cc b/libstdc++-v3/testsuite/experimental/filesystem/operations/last_write_time.cc
index dee55a5..74be4f9 100644
--- a/libstdc++-v3/testsuite/experimental/filesystem/operations/last_write_time.cc
+++ b/libstdc++-v3/testsuite/experimental/filesystem/operations/last_write_time.cc
@@ -35,6 +35,8 @@
 void
 test01()
 {
+  // read times
+
   using time_type = std::experimental::filesystem::file_time_type;
 
   auto p = __gnu_test::nonexistent_path();
@@ -103,8 +105,46 @@ test01()
 #endif
 }
 
+void
+test02()
+{
+  // write times
+
+  using time_type = std::experimental::filesystem::file_time_type;
+
+  __gnu_test::scoped_file f;
+  std::error_code ec;
+  time_type time;
+
+  time = last_write_time(f.path);
+  last_write_time(f.path, time, ec);
+  VERIFY( !ec );
+  VERIFY( last_write_time(f.path) == time );
+
+  time -= std::chrono::milliseconds(1000 * 60 * 10 + 15);
+  last_write_time(f.path, time, ec);
+  VERIFY( !ec );
+  VERIFY( last_write_time(f.path) == time );
+
+  time += std::chrono::milliseconds(1000 * 60 * 20 + 15);
+  last_write_time(f.path, time, ec);
+  VERIFY( !ec );
+  VERIFY( last_write_time(f.path) == time );
+
+  time = time_type();
+  last_write_time(f.path, time, ec);
+  VERIFY( !ec );
+  VERIFY( last_write_time(f.path) == time );
+
+  time -= std::chrono::milliseconds(1000 * 60 * 10 + 15);
+  last_write_time(f.path, time, ec);
+  VERIFY( !ec );
+  VERIFY( last_write_time(f.path) == time );
+}
+
 int
 main()
 {
   test01();
+  test02();
 }

commit 7115df4e3a4da1f756ca7419a841e5d656cc83f3
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Mon Oct 24 19:52:57 2016 +0100

    Fix error handling in copy_file and equivalent
    
    	* src/filesystem/ops.cc (do_copy_file): Report an error if source or
    	destination is not a regular file (LWG 2712).
    	(equivalent): Fix error handling and result when only one file exists.
    	* testsuite/experimental/filesystem/operations/copy.cc: Remove files
    	created by tests. Test copying directories.
    	* testsuite/experimental/filesystem/operations/copy_file.cc: Remove
    	files created by tests.
    	* testsuite/experimental/filesystem/operations/equivalent.cc: New.
    	* testsuite/experimental/filesystem/operations/is_empty.cc: New.
    	* testsuite/experimental/filesystem/operations/read_symlink.cc: Remove
    	file created by test.
    	* testsuite/experimental/filesystem/operations/remove_all.cc: New.
    	* testsuite/util/testsuite_fs.h (~scoped_file): Only try to remove
    	file if path is non-empty, to support removal by other means.

diff --git a/libstdc++-v3/src/filesystem/ops.cc b/libstdc++-v3/src/filesystem/ops.cc
index d9a12df..fcf747e 100644
--- a/libstdc++-v3/src/filesystem/ops.cc
+++ b/libstdc++-v3/src/filesystem/ops.cc
@@ -350,6 +350,8 @@ namespace
 	  from_st = &st2;
       }
     f = make_file_status(*from_st);
+    // _GLIBCXX_RESOLVE_LIB_DEFECTS
+    // 2712. copy_file() has a number of unspecified error conditions
     if (!is_regular_file(f))
       {
 	ec = std::make_error_code(std::errc::not_supported);
@@ -360,8 +362,13 @@ namespace
 
     if (exists(t))
       {
-	if (!is_other(t) && !is_other(f)
-	    && to_st->st_dev == from_st->st_dev
+	if (!is_regular_file(t))
+	  {
+	    ec = std::make_error_code(std::errc::not_supported);
+	    return false;
+	  }
+
+	if (to_st->st_dev == from_st->st_dev
 	    && to_st->st_ino == from_st->st_ino)
 	  {
 	    ec = std::make_error_code(std::errc::file_exists);
@@ -912,7 +919,7 @@ fs::equivalent(const path& p1, const path& p2)
 {
   error_code ec;
   auto result = equivalent(p1, p2, ec);
-  if (ec.value())
+  if (ec)
     _GLIBCXX_THROW_OR_ABORT(filesystem_error("cannot check file equivalence",
 	  p1, p2, ec));
   return result;
@@ -922,25 +929,42 @@ bool
 fs::equivalent(const path& p1, const path& p2, error_code& ec) noexcept
 {
 #ifdef _GLIBCXX_HAVE_SYS_STAT_H
+  int err = 0;
+  file_status s1, s2;
   stat_type st1, st2;
-  if (::stat(p1.c_str(), &st1) == 0 && ::stat(p2.c_str(), &st2) == 0)
+  if (::stat(p1.c_str(), &st1) == 0)
+    s1 = make_file_status(st1);
+  else if (is_not_found_errno(errno))
+    s1.type(file_type::not_found);
+  else
+    err = errno;
+
+  if (::stat(p2.c_str(), &st2) == 0)
+    s2 = make_file_status(st2);
+  else if (is_not_found_errno(errno))
+    s2.type(file_type::not_found);
+  else
+    err = errno;
+
+  if (exists(s1) && exists(s2))
     {
-      file_status s1 = make_file_status(st1);
-      file_status s2 = make_file_status(st2);
       if (is_other(s1) && is_other(s2))
 	{
 	  ec = std::make_error_code(std::errc::not_supported);
 	  return false;
 	}
       ec.clear();
+      if (is_other(s1) || is_other(s2))
+	return false;
       return st1.st_dev == st2.st_dev && st1.st_ino == st2.st_ino;
     }
-  else if (is_not_found_errno(errno))
-    {
-      ec = std::make_error_code(std::errc::no_such_file_or_directory);
-      return false;
-    }
-  ec.assign(errno, std::generic_category());
+  else if (!exists(s1) && !exists(s2))
+    ec = std::make_error_code(std::errc::no_such_file_or_directory);
+  else if (err)
+    ec.assign(err, std::generic_category());
+  else
+    ec.clear();
+  return false;
 #else
   ec = std::make_error_code(std::errc::not_supported);
 #endif
diff --git a/libstdc++-v3/testsuite/experimental/filesystem/operations/copy.cc b/libstdc++-v3/testsuite/experimental/filesystem/operations/copy.cc
index 2cfb1c1..7173788 100644
--- a/libstdc++-v3/testsuite/experimental/filesystem/operations/copy.cc
+++ b/libstdc++-v3/testsuite/experimental/filesystem/operations/copy.cc
@@ -128,6 +128,9 @@ test03()
   fs::copy(from, to);
   VERIFY( fs::exists(to) );
   VERIFY( fs::file_size(to) == fs::file_size(from) );
+
+  remove(from);
+  remove(to);
 }
 
 // Test is_directory(f) case.
@@ -138,6 +141,37 @@ test04()
   auto to = __gnu_test::nonexistent_path();
   std::error_code ec;
 
+  create_directories(from/"a/b/c");
+
+  {
+    __gnu_test::scoped_file f(to);
+    copy(from, to, ec);
+    VERIFY( ec );
+  }
+
+  __gnu_test::scoped_file f1(from/"a/f1");
+  std::ofstream{f1.path} << "file one";
+  __gnu_test::scoped_file f2(from/"a/b/f2");
+  std::ofstream{f2.path} << "file two";
+
+  copy(from, to, ec);
+  VERIFY( !ec );
+  VERIFY( exists(to) && is_empty(to) );
+  remove(to);
+
+  copy(from, to, fs::copy_options::recursive, ec);
+  VERIFY( !ec );
+  VERIFY( exists(to) && !is_empty(to) );
+  VERIFY( is_regular_file(to/"a/f1") && !is_empty(to/"a/f1") );
+  VERIFY( file_size(from/"a/f1") == file_size(to/"a/f1") );
+  VERIFY( is_regular_file(to/"a/b/f2") && !is_empty(to/"a/b/f2") );
+  VERIFY( file_size(from/"a/b/f2") == file_size(to/"a/b/f2") );
+  VERIFY( is_directory(to/"a/b/c") && is_empty(to/"a/b/c") );
+
+  f1.path.clear();
+  f2.path.clear();
+  remove_all(from, ec);
+  remove_all(to, ec);
 }
 
 // Test no-op cases.
diff --git a/libstdc++-v3/testsuite/experimental/filesystem/operations/copy_file.cc b/libstdc++-v3/testsuite/experimental/filesystem/operations/copy_file.cc
index 9cb09d3..1e487cd 100644
--- a/libstdc++-v3/testsuite/experimental/filesystem/operations/copy_file.cc
+++ b/libstdc++-v3/testsuite/experimental/filesystem/operations/copy_file.cc
@@ -73,6 +73,9 @@ test01()
   VERIFY( !ec );
   VERIFY( exists(to) );
   VERIFY( file_size(to) == file_size(from) );
+
+  remove(from);
+  remove(to);
 }
 
 int
diff --git a/libstdc++-v3/testsuite/experimental/filesystem/operations/equivalent.cc b/libstdc++-v3/testsuite/experimental/filesystem/operations/equivalent.cc
new file mode 100644
index 0000000..77ed054
--- /dev/null
+++ b/libstdc++-v3/testsuite/experimental/filesystem/operations/equivalent.cc
@@ -0,0 +1,74 @@
+// Copyright (C) 2016 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-lstdc++fs" }
+// { dg-do run { target c++11 } }
+// { dg-require-filesystem-ts "" }
+
+#include <experimental/filesystem>
+#include <testsuite_fs.h>
+#include <testsuite_hooks.h>
+
+namespace fs = std::experimental::filesystem;
+
+
+void
+test01()
+{
+  auto p1 = __gnu_test::nonexistent_path();
+  auto p2 = __gnu_test::nonexistent_path();
+  std::error_code ec;
+  bool result;
+
+  result = equivalent(p1, p2, ec);
+  VERIFY( ec );
+  VERIFY( !result );
+  const auto bad_ec = ec;
+
+  __gnu_test::scoped_file f1(p1);
+  result = equivalent(p1, p2, ec);
+  VERIFY( !ec );
+  VERIFY( !result );
+
+  __gnu_test::scoped_file f2(p2);
+  ec = bad_ec;
+  result = equivalent(p1, p2, ec);
+  VERIFY( !ec );
+  VERIFY( !result );
+
+  auto p3 = __gnu_test::nonexistent_path();
+  create_hard_link(p1, p3, ec);
+  if (ec)
+    return;  // hard links not supported
+  __gnu_test::scoped_file f3(p3, __gnu_test::scoped_file::adopt_file);
+
+  ec = bad_ec;
+  result = equivalent(p1, p3, ec);
+  VERIFY( !ec );
+  VERIFY( result );
+
+  ec = bad_ec;
+  result = equivalent(p2, p3, ec);
+  VERIFY( !ec );
+  VERIFY( !result );
+}
+
+int
+main()
+{
+  test01();
+}
diff --git a/libstdc++-v3/testsuite/experimental/filesystem/operations/is_empty.cc b/libstdc++-v3/testsuite/experimental/filesystem/operations/is_empty.cc
new file mode 100644
index 0000000..d35967a
--- /dev/null
+++ b/libstdc++-v3/testsuite/experimental/filesystem/operations/is_empty.cc
@@ -0,0 +1,109 @@
+// Copyright (C) 2016 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-lstdc++fs" }
+// { dg-do run { target c++11 } }E
+// { dg-require-filesystem-ts "" }
+
+#include <experimental/filesystem>
+#include <testsuite_hooks.h>
+#include <testsuite_fs.h>
+
+namespace fs = std::experimental::filesystem;
+
+void
+test01()
+{
+  auto p = __gnu_test::nonexistent_path();
+  create_directory(p);
+  permissions(p, fs::perms::none);
+  std::error_code ec, ec2;
+
+  bool result = fs::is_empty(p, ec);
+  VERIFY( ec == std::make_error_code(std::errc::permission_denied) );
+  VERIFY( !result );
+
+  try {
+    fs::is_empty(p);
+  } catch (const fs::filesystem_error& e) {
+    ec2 = e.code();
+  }
+  VERIFY( ec2 == ec );
+
+  result = fs::is_empty(p/"f", ec);
+  VERIFY( ec == std::make_error_code(std::errc::permission_denied) );
+  VERIFY( !result );
+
+  try {
+    fs::is_empty(p/"f");
+  } catch (const fs::filesystem_error& e) {
+    ec2 = e.code();
+  }
+  VERIFY( ec2 == ec );
+
+  permissions(p, fs::perms::owner_all, ec);
+  remove_all(p, ec);
+}
+
+void
+test02()
+{
+  auto p = __gnu_test::nonexistent_path();
+  create_directory(p);
+  std::error_code ec, bad_ec = make_error_code(std::errc::invalid_argument);
+  bool empty;
+
+  ec = bad_ec;
+  empty = is_empty(p, ec);
+  VERIFY( !ec );
+  VERIFY( empty );
+  empty = is_empty(p);
+  VERIFY( empty );
+
+  __gnu_test::scoped_file f(p/"f");
+  ec = bad_ec;
+  empty = is_empty(f.path, ec);
+  VERIFY( !ec );
+  VERIFY( empty );
+  empty = is_empty(f.path);
+  VERIFY( empty );
+
+  std::ofstream{f.path.native()} << "data";
+  ec = bad_ec;
+  empty = is_empty(p, ec);
+  VERIFY( !ec );
+  VERIFY( !empty );
+  empty = is_empty(p);
+  VERIFY( !empty );
+
+  ec = bad_ec;
+  empty = is_empty(p, ec);
+  VERIFY( !ec );
+  VERIFY( !empty );
+  empty = is_empty(p);
+  VERIFY( !empty );
+
+  f.path.clear();
+  remove_all(p, ec);
+}
+
+int
+main()
+{
+  test01();
+  test02();
+}
diff --git a/libstdc++-v3/testsuite/experimental/filesystem/operations/read_symlink.cc b/libstdc++-v3/testsuite/experimental/filesystem/operations/read_symlink.cc
index c4137bd..c67b318 100644
--- a/libstdc++-v3/testsuite/experimental/filesystem/operations/read_symlink.cc
+++ b/libstdc++-v3/testsuite/experimental/filesystem/operations/read_symlink.cc
@@ -40,6 +40,8 @@ test01()
   auto result = read_symlink(p, ec);
   VERIFY( !ec );
   VERIFY( result == tgt );
+
+  remove(p);
 }
 
 int
diff --git a/libstdc++-v3/testsuite/experimental/filesystem/operations/remove_all.cc b/libstdc++-v3/testsuite/experimental/filesystem/operations/remove_all.cc
new file mode 100644
index 0000000..57d15af
--- /dev/null
+++ b/libstdc++-v3/testsuite/experimental/filesystem/operations/remove_all.cc
@@ -0,0 +1,92 @@
+// Copyright (C) 2016 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-lstdc++fs" }
+// { dg-do run { target c++11 } }
+// { dg-require-filesystem-ts "" }
+
+#include <experimental/filesystem>
+#include <testsuite_hooks.h>
+#include <testsuite_fs.h>
+
+namespace fs = std::experimental::filesystem;
+
+void
+test01()
+{
+  std::error_code ec;
+  std::uintmax_t n;
+
+  n = fs::remove_all("", ec);
+  VERIFY( ec );
+  VERIFY( n == std::uintmax_t(-1) );
+
+  auto p = __gnu_test::nonexistent_path();
+  ec.clear();
+  n = remove_all(p, ec);
+  VERIFY( ec );
+  VERIFY( n == std::uintmax_t(-1) );
+
+  const auto bad_ec = ec;
+  auto link = __gnu_test::nonexistent_path();
+  create_symlink(p, link);  // dangling symlink
+  ec = bad_ec;
+  n = remove_all(link, ec);
+  VERIFY( !ec );
+  VERIFY( n == 1 );
+  VERIFY( !exists(symlink_status(link)) ); // DR 2721
+
+  __gnu_test::scoped_file f(p);
+  create_symlink(p, link);
+  ec = bad_ec;
+  n = remove_all(link, ec);
+  VERIFY( !ec );
+  VERIFY( n == 1 );
+  VERIFY( !exists(symlink_status(link)) );  // The symlink is removed, but
+  VERIFY( exists(p) );                      // its target is not.
+
+  auto dir = __gnu_test::nonexistent_path();
+  create_directories(dir/"a/b/c");
+  ec = bad_ec;
+  n = remove_all(dir/"a", ec);
+  VERIFY( !ec );
+  VERIFY( n == 3 );
+  VERIFY( exists(dir) );
+  VERIFY( !exists(dir/"a") );
+
+  create_directories(dir/"a/b/c");
+  __gnu_test::scoped_file a1(dir/"a/1");
+  __gnu_test::scoped_file a2(dir/"a/2");
+  __gnu_test::scoped_file b1(dir/"a/b/1");
+  __gnu_test::scoped_file b2(dir/"a/b/2");
+  ec = bad_ec;
+  n = remove_all(dir, ec);
+  VERIFY( !ec );
+  VERIFY( n == 8 );
+  VERIFY( !exists(dir) );
+
+  a1.path.clear();
+  a2.path.clear();
+  b1.path.clear();
+  b2.path.clear();
+}
+
+int
+main()
+{
+  test01();
+}
diff --git a/libstdc++-v3/testsuite/util/testsuite_fs.h b/libstdc++-v3/testsuite/util/testsuite_fs.h
index 5b36670..7d9b20c 100644
--- a/libstdc++-v3/testsuite/util/testsuite_fs.h
+++ b/libstdc++-v3/testsuite/util/testsuite_fs.h
@@ -107,7 +107,7 @@ namespace __gnu_test
 
     scoped_file(path_type p, adopt_file_t) : path(p) { }
 
-    ~scoped_file() { remove(path); }
+    ~scoped_file() { if (!path.empty()) remove(path); }
 
     path_type path;
   };

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Five patches for std::experimental::filesystem
  2016-10-25 15:32 ` Jonathan Wakely
@ 2016-10-26  7:24   ` Christophe Lyon
  2016-10-27 13:35     ` Jonathan Wakely
  0 siblings, 1 reply; 8+ messages in thread
From: Christophe Lyon @ 2016-10-26  7:24 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: libstdc++, gcc-patches

Hi Jonathan,

On 25 October 2016 at 17:32, Jonathan Wakely <jwakely@redhat.com> wrote:
> Two more fixes for the filesystem TS, and improved tests.
>
>   Handle negative times in filesystem::last_write_time
>           * src/filesystem/ops.cc
>        (last_write_time(const path&, file_time_type, error_code&)): Handle
>        negative times correctly.
>        * testsuite/experimental/filesystem/operations/last_write_time.cc:
>        Test writing file times.
>
>    Fix error handling in copy_file and equivalent
>           * src/filesystem/ops.cc (do_copy_file): Report an error if source
> or
>        destination is not a regular file (LWG 2712).
>        (equivalent): Fix error handling and result when only one file
> exists.
>        * testsuite/experimental/filesystem/operations/copy.cc: Remove files
>        created by tests. Test copying directories.
>        * testsuite/experimental/filesystem/operations/copy_file.cc: Remove
>        files created by tests.
>        * testsuite/experimental/filesystem/operations/equivalent.cc: New.
>        * testsuite/experimental/filesystem/operations/is_empty.cc: New.
>        * testsuite/experimental/filesystem/operations/read_symlink.cc:
> Remove
>        file created by test.
>        * testsuite/experimental/filesystem/operations/remove_all.cc: New.
>        * testsuite/util/testsuite_fs.h (~scoped_file): Only try to remove
>        file if path is non-empty, to support removal by other means.
>
> Tested x86_64-linux, committed to trunk.
>
>
I can see failures in
experimental/filesystem/operations/last_write_time.cc after your
committed this patch:
/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libstdc++-v3/testsuite/experimental/filesystem/operations/last_write_time.cc:127:
void test02(): Assertion 'last_write_time(f.path) == time' failed.
on arm*linux* and aarch64*linux* targets.

Christophe

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Five patches for std::experimental::filesystem
  2016-10-24 16:50 [PATCH] Five patches for std::experimental::filesystem Jonathan Wakely
  2016-10-25 15:32 ` Jonathan Wakely
@ 2016-10-26 13:34 ` Jonathan Wakely
  1 sibling, 0 replies; 8+ messages in thread
From: Jonathan Wakely @ 2016-10-26 13:34 UTC (permalink / raw)
  To: libstdc++, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 939 bytes --]

On 24/10/16 17:50 +0100, Jonathan Wakely wrote:
>   Make directory iterators become end iterator on error
>       * src/filesystem/dir.cc (open_dir): Return same value for errors
>       whether ignored or not.
>       (_Dir::advance(error_code*, directory_options)): Return false on
>       error.
>       (directory_iterator(const path&, directory_options, error_code*)):
>       Create end iterator on error (LWG 2723).
>       (recursive_directory_iterator(const path&, directory_options,
>       error_code*)): Likewise.
>       * testsuite/experimental/filesystem/iterators/directory_iterator.cc:
>       Update expected behaviour on error.
>       * testsuite/experimental/filesystem/iterators/
>       recursive_directory_iterator.cc: Likewise.

I missed the case where recursing into a sub-directory fails, and that
happened to be the one bit of the test where I didn't add a new check.

Tested x86_64-linux, committed to trunk.


[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 1670 bytes --]

commit a38bbf551b4f49e47fe1da03b779edba66240d5f
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Wed Oct 26 13:34:15 2016 +0100

    Fix error handling in recursive_directory_iterator::increment
    
    	* src/filesystem/dir.cc (recursive_directory_iterator::increment):
    	Reset state on error.
    	* testsuite/experimental/filesystem/iterators/
    	recursive_directory_iterator.cc: Check state after increment error.

diff --git a/libstdc++-v3/src/filesystem/dir.cc b/libstdc++-v3/src/filesystem/dir.cc
index 4640d75..bcd7dd0 100644
--- a/libstdc++-v3/src/filesystem/dir.cc
+++ b/libstdc++-v3/src/filesystem/dir.cc
@@ -343,7 +343,10 @@ fs::recursive_directory_iterator::increment(error_code& ec) noexcept
     {
       _Dir dir = open_dir(top.entry.path(), _M_options, &ec);
       if (ec)
-	return *this;
+	{
+	  _M_dirs.reset();
+	  return *this;
+	}
       if (dir.dirp)
 	  _M_dirs->push(std::move(dir));
     }
diff --git a/libstdc++-v3/testsuite/experimental/filesystem/iterators/recursive_directory_iterator.cc b/libstdc++-v3/testsuite/experimental/filesystem/iterators/recursive_directory_iterator.cc
index b41c394..79aa178 100644
--- a/libstdc++-v3/testsuite/experimental/filesystem/iterators/recursive_directory_iterator.cc
+++ b/libstdc++-v3/testsuite/experimental/filesystem/iterators/recursive_directory_iterator.cc
@@ -81,6 +81,7 @@ test01()
   VERIFY( iter->path() == p/"d1/d2" );
   iter.increment(ec);  // should fail to recurse into p/d1/d2
   VERIFY( ec );
+  VERIFY( iter == fs::recursive_directory_iterator() );
 
   // Test inaccessible sub-directory, skipping permission denied.
   iter = fs::recursive_directory_iterator(p, opts, ec);

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Five patches for std::experimental::filesystem
  2016-10-26  7:24   ` Christophe Lyon
@ 2016-10-27 13:35     ` Jonathan Wakely
  2016-11-02  9:09       ` Christophe Lyon
  0 siblings, 1 reply; 8+ messages in thread
From: Jonathan Wakely @ 2016-10-27 13:35 UTC (permalink / raw)
  To: Christophe Lyon; +Cc: libstdc++, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 2231 bytes --]

On 26/10/16 09:24 +0200, Christophe Lyon wrote:
>Hi Jonathan,
>
>On 25 October 2016 at 17:32, Jonathan Wakely <jwakely@redhat.com> wrote:
>> Two more fixes for the filesystem TS, and improved tests.
>>
>>   Handle negative times in filesystem::last_write_time
>>           * src/filesystem/ops.cc
>>        (last_write_time(const path&, file_time_type, error_code&)): Handle
>>        negative times correctly.
>>        * testsuite/experimental/filesystem/operations/last_write_time.cc:
>>        Test writing file times.
>>
>>    Fix error handling in copy_file and equivalent
>>           * src/filesystem/ops.cc (do_copy_file): Report an error if source
>> or
>>        destination is not a regular file (LWG 2712).
>>        (equivalent): Fix error handling and result when only one file
>> exists.
>>        * testsuite/experimental/filesystem/operations/copy.cc: Remove files
>>        created by tests. Test copying directories.
>>        * testsuite/experimental/filesystem/operations/copy_file.cc: Remove
>>        files created by tests.
>>        * testsuite/experimental/filesystem/operations/equivalent.cc: New.
>>        * testsuite/experimental/filesystem/operations/is_empty.cc: New.
>>        * testsuite/experimental/filesystem/operations/read_symlink.cc:
>> Remove
>>        file created by test.
>>        * testsuite/experimental/filesystem/operations/remove_all.cc: New.
>>        * testsuite/util/testsuite_fs.h (~scoped_file): Only try to remove
>>        file if path is non-empty, to support removal by other means.
>>
>> Tested x86_64-linux, committed to trunk.
>>
>>
>I can see failures in
>experimental/filesystem/operations/last_write_time.cc after your
>committed this patch:
>/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libstdc++-v3/testsuite/experimental/filesystem/operations/last_write_time.cc:127:
>void test02(): Assertion 'last_write_time(f.path) == time' failed.
>on arm*linux* and aarch64*linux* targets.

That test will fail for targets where _GLIBCXX_USE_UTIMENSAT is not
defined, as they use utime() instead which only supports second
granularity.

This should solve it, by only checking that the file times are within
one second of the expected value.

Tested x86_64-linux, committed to trunk.

[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 9716 bytes --]

commit 9b3f71e225ae45ccc1a5a78fee05d4ed9a969e8e
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Oct 27 11:48:00 2016 +0100

    Adjust precision of filesystem::last_write_time tests
    
    	* testsuite/experimental/filesystem/iterators/directory_iterator.cc:
    	Use end() function to get end iterator.
    	* testsuite/experimental/filesystem/iterators/pop.cc: Remove printf
    	statements that were present for debugging.
    	* testsuite/experimental/filesystem/iterators/
    	recursive_directory_iterator.cc: Use end() function to get end
    	iterator.
    	* testsuite/experimental/filesystem/operations/last_write_time.cc:
    	Only require file timestamps to be accurate to one second.

diff --git a/libstdc++-v3/testsuite/experimental/filesystem/iterators/directory_iterator.cc b/libstdc++-v3/testsuite/experimental/filesystem/iterators/directory_iterator.cc
index 5788700..d1f2c54 100644
--- a/libstdc++-v3/testsuite/experimental/filesystem/iterators/directory_iterator.cc
+++ b/libstdc++-v3/testsuite/experimental/filesystem/iterators/directory_iterator.cc
@@ -34,14 +34,14 @@ test01()
   const auto p = __gnu_test::nonexistent_path();
   fs::directory_iterator iter(p, ec);
   VERIFY( ec );
-  VERIFY( iter == fs::directory_iterator() );
+  VERIFY( iter == end(iter) );
 
   // Test empty directory.
   create_directory(p, fs::current_path(), ec);
   VERIFY( !ec );
   iter = fs::directory_iterator(p, ec);
   VERIFY( !ec );
-  VERIFY( iter == fs::directory_iterator() );
+  VERIFY( iter == end(iter) );
 
   // Test non-empty directory.
   create_directory_symlink(p, p / "l", ec);
@@ -51,20 +51,20 @@ test01()
   VERIFY( iter != fs::directory_iterator() );
   VERIFY( iter->path() == p/"l" );
   ++iter;
-  VERIFY( iter == fs::directory_iterator() );
+  VERIFY( iter == end(iter) );
 
   // Test inaccessible directory.
   permissions(p, fs::perms::none, ec);
   VERIFY( !ec );
   iter = fs::directory_iterator(p, ec);
   VERIFY( ec );
-  VERIFY( iter == fs::directory_iterator() );
+  VERIFY( iter == end(iter) );
 
   // Test inaccessible directory, skipping permission denied.
   const auto opts = fs::directory_options::skip_permission_denied;
   iter = fs::directory_iterator(p, opts, ec);
   VERIFY( !ec );
-  VERIFY( iter == fs::directory_iterator() );
+  VERIFY( iter == end(iter) );
 
   permissions(p, fs::perms::owner_all, ec);
   remove_all(p, ec);
@@ -82,12 +82,12 @@ test02()
   // Test post-increment (libstdc++/71005)
   auto iter = fs::directory_iterator(p, ec);
   VERIFY( !ec );
-  VERIFY( iter != fs::directory_iterator() );
+  VERIFY( iter != end(iter) );
   const auto entry1 = *iter;
   const auto entry2 = *iter++;
   VERIFY( entry1 == entry2 );
   VERIFY( entry1.path() == p/"l" );
-  VERIFY( iter == fs::directory_iterator() );
+  VERIFY( iter == end(iter) );
 
   remove_all(p, ec);
 }
diff --git a/libstdc++-v3/testsuite/experimental/filesystem/iterators/pop.cc b/libstdc++-v3/testsuite/experimental/filesystem/iterators/pop.cc
index d247ab4..9306c03 100644
--- a/libstdc++-v3/testsuite/experimental/filesystem/iterators/pop.cc
+++ b/libstdc++-v3/testsuite/experimental/filesystem/iterators/pop.cc
@@ -84,14 +84,10 @@ test03()
     std::advance(dir, i);
     int expected_depth = i;
     VERIFY( dir.depth() == expected_depth );
-    __builtin_printf("%d %d %s\n", i, dir.depth(), dir->path().c_str());
     dir.pop(ec);
     VERIFY( !ec );
     if (dir != end(dir))
-    {
-    __builtin_printf("%d %d %s\n", i, dir.depth(), dir->path().c_str());
       VERIFY( dir.depth() == (expected_depth - 1) );
-    }
 
     dir = fs::recursive_directory_iterator(p);
     std::advance(dir, i);
diff --git a/libstdc++-v3/testsuite/experimental/filesystem/iterators/recursive_directory_iterator.cc b/libstdc++-v3/testsuite/experimental/filesystem/iterators/recursive_directory_iterator.cc
index 79aa178..3dc7ba2 100644
--- a/libstdc++-v3/testsuite/experimental/filesystem/iterators/recursive_directory_iterator.cc
+++ b/libstdc++-v3/testsuite/experimental/filesystem/iterators/recursive_directory_iterator.cc
@@ -34,39 +34,39 @@ test01()
   const auto p = __gnu_test::nonexistent_path();
   fs::recursive_directory_iterator iter(p, ec);
   VERIFY( ec );
-  VERIFY( iter == fs::recursive_directory_iterator() );
+  VERIFY( iter == end(iter) );
 
   // Test empty directory.
   create_directory(p, fs::current_path(), ec);
   VERIFY( !ec );
   iter = fs::recursive_directory_iterator(p, ec);
   VERIFY( !ec );
-  VERIFY( iter == fs::recursive_directory_iterator() );
+  VERIFY( iter == end(iter) );
 
   // Test non-empty directory.
   create_directories(p / "d1/d2");
   VERIFY( !ec );
   iter = fs::recursive_directory_iterator(p, ec);
   VERIFY( !ec );
-  VERIFY( iter != fs::recursive_directory_iterator() );
+  VERIFY( iter != end(iter) );
   VERIFY( iter->path() == p/"d1" );
   ++iter;
   VERIFY( iter->path() == p/"d1/d2" );
   ++iter;
-  VERIFY( iter == fs::recursive_directory_iterator() );
+  VERIFY( iter == end(iter) );
 
   // Test inaccessible directory.
   permissions(p, fs::perms::none, ec);
   VERIFY( !ec );
   iter = fs::recursive_directory_iterator(p, ec);
   VERIFY( ec );
-  VERIFY( iter == fs::recursive_directory_iterator() );
+  VERIFY( iter == end(iter) );
 
   // Test inaccessible directory, skipping permission denied.
   const auto opts = fs::directory_options::skip_permission_denied;
   iter = fs::recursive_directory_iterator(p, opts, ec);
   VERIFY( !ec );
-  VERIFY( iter == fs::recursive_directory_iterator() );
+  VERIFY( iter == end(iter) );
 
   // Test inaccessible sub-directory.
   permissions(p, fs::perms::owner_all, ec);
@@ -75,24 +75,24 @@ test01()
   VERIFY( !ec );
   iter = fs::recursive_directory_iterator(p, ec);
   VERIFY( !ec );
-  VERIFY( iter != fs::recursive_directory_iterator() );
+  VERIFY( iter != end(iter) );
   VERIFY( iter->path() == p/"d1" );
   ++iter;              // should recurse into d1
   VERIFY( iter->path() == p/"d1/d2" );
   iter.increment(ec);  // should fail to recurse into p/d1/d2
   VERIFY( ec );
-  VERIFY( iter == fs::recursive_directory_iterator() );
+  VERIFY( iter == end(iter) );
 
   // Test inaccessible sub-directory, skipping permission denied.
   iter = fs::recursive_directory_iterator(p, opts, ec);
   VERIFY( !ec );
-  VERIFY( iter != fs::recursive_directory_iterator() );
+  VERIFY( iter != end(iter) );
   VERIFY( iter->path() == p/"d1" );
   ++iter;              // should recurse into d1
   VERIFY( iter->path() == p/"d1/d2" );
   iter.increment(ec);  // should fail to recurse into p/d1/d2, so skip it
   VERIFY( !ec );
-  VERIFY( iter == fs::recursive_directory_iterator() );
+  VERIFY( iter == end(iter) );
 
   permissions(p/"d1/d2", fs::perms::owner_all, ec);
   remove_all(p, ec);
@@ -109,7 +109,7 @@ test02()
   // Test post-increment (libstdc++/71005)
   auto iter = fs::recursive_directory_iterator(p, ec);
   VERIFY( !ec );
-  VERIFY( iter != fs::recursive_directory_iterator() );
+  VERIFY( iter != end(iter) );
   const auto entry1 = *iter;
   const auto entry2 = *iter++;
   VERIFY( entry1 == entry2 );
@@ -118,7 +118,7 @@ test02()
   const auto entry4 = *iter++;
   VERIFY( entry3 == entry4 );
   VERIFY( entry3.path() == p/"d1/d2" );
-  VERIFY( iter == fs::recursive_directory_iterator() );
+  VERIFY( iter == end(iter) );
 
   remove_all(p, ec);
 }
@@ -145,7 +145,7 @@ test04()
 {
   // libstdc++/71004
   const fs::recursive_directory_iterator it;
-  VERIFY( it == fs::recursive_directory_iterator() );
+  VERIFY( it == end(it) );
 }
 
 void
diff --git a/libstdc++-v3/testsuite/experimental/filesystem/operations/last_write_time.cc b/libstdc++-v3/testsuite/experimental/filesystem/operations/last_write_time.cc
index 74be4f9..a60a25f 100644
--- a/libstdc++-v3/testsuite/experimental/filesystem/operations/last_write_time.cc
+++ b/libstdc++-v3/testsuite/experimental/filesystem/operations/last_write_time.cc
@@ -32,13 +32,13 @@
 # include <utime.h>
 #endif
 
+using time_type = std::experimental::filesystem::file_time_type;
+
 void
 test01()
 {
   // read times
 
-  using time_type = std::experimental::filesystem::file_time_type;
-
   auto p = __gnu_test::nonexistent_path();
   std::error_code ec;
   time_type mtime = last_write_time(p, ec);
@@ -105,13 +105,19 @@ test01()
 #endif
 }
 
+bool approx_equal(time_type file_time, time_type expected)
+{
+  auto delta = expected - file_time;
+  if (delta < delta.zero())
+    delta = -delta;
+  return delta < std::chrono::seconds(1);
+}
+
 void
 test02()
 {
   // write times
 
-  using time_type = std::experimental::filesystem::file_time_type;
-
   __gnu_test::scoped_file f;
   std::error_code ec;
   time_type time;
@@ -119,27 +125,27 @@ test02()
   time = last_write_time(f.path);
   last_write_time(f.path, time, ec);
   VERIFY( !ec );
-  VERIFY( last_write_time(f.path) == time );
+  VERIFY( approx_equal(last_write_time(f.path), time) );
 
   time -= std::chrono::milliseconds(1000 * 60 * 10 + 15);
   last_write_time(f.path, time, ec);
   VERIFY( !ec );
-  VERIFY( last_write_time(f.path) == time );
+  VERIFY( approx_equal(last_write_time(f.path), time) );
 
   time += std::chrono::milliseconds(1000 * 60 * 20 + 15);
   last_write_time(f.path, time, ec);
   VERIFY( !ec );
-  VERIFY( last_write_time(f.path) == time );
+  VERIFY( approx_equal(last_write_time(f.path), time) );
 
   time = time_type();
   last_write_time(f.path, time, ec);
   VERIFY( !ec );
-  VERIFY( last_write_time(f.path) == time );
+  VERIFY( approx_equal(last_write_time(f.path), time) );
 
   time -= std::chrono::milliseconds(1000 * 60 * 10 + 15);
   last_write_time(f.path, time, ec);
   VERIFY( !ec );
-  VERIFY( last_write_time(f.path) == time );
+  VERIFY( approx_equal(last_write_time(f.path), time) );
 }
 
 int

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Five patches for std::experimental::filesystem
  2016-10-27 13:35     ` Jonathan Wakely
@ 2016-11-02  9:09       ` Christophe Lyon
  2016-11-04 14:31         ` Christophe Lyon
  0 siblings, 1 reply; 8+ messages in thread
From: Christophe Lyon @ 2016-11-02  9:09 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: libstdc++, gcc-patches

On 27 October 2016 at 15:34, Jonathan Wakely <jwakely@redhat.com> wrote:
> On 26/10/16 09:24 +0200, Christophe Lyon wrote:
>>
>> Hi Jonathan,
>>
>> On 25 October 2016 at 17:32, Jonathan Wakely <jwakely@redhat.com> wrote:
>>>
>>> Two more fixes for the filesystem TS, and improved tests.
>>>
>>>   Handle negative times in filesystem::last_write_time
>>>           * src/filesystem/ops.cc
>>>        (last_write_time(const path&, file_time_type, error_code&)):
>>> Handle
>>>        negative times correctly.
>>>        * testsuite/experimental/filesystem/operations/last_write_time.cc:
>>>        Test writing file times.
>>>
>>>    Fix error handling in copy_file and equivalent
>>>           * src/filesystem/ops.cc (do_copy_file): Report an error if
>>> source
>>> or
>>>        destination is not a regular file (LWG 2712).
>>>        (equivalent): Fix error handling and result when only one file
>>> exists.
>>>        * testsuite/experimental/filesystem/operations/copy.cc: Remove
>>> files
>>>        created by tests. Test copying directories.
>>>        * testsuite/experimental/filesystem/operations/copy_file.cc:
>>> Remove
>>>        files created by tests.
>>>        * testsuite/experimental/filesystem/operations/equivalent.cc: New.
>>>        * testsuite/experimental/filesystem/operations/is_empty.cc: New.
>>>        * testsuite/experimental/filesystem/operations/read_symlink.cc:
>>> Remove
>>>        file created by test.
>>>        * testsuite/experimental/filesystem/operations/remove_all.cc: New.
>>>        * testsuite/util/testsuite_fs.h (~scoped_file): Only try to remove
>>>        file if path is non-empty, to support removal by other means.
>>>
>>> Tested x86_64-linux, committed to trunk.
>>>
>>>
>> I can see failures in
>> experimental/filesystem/operations/last_write_time.cc after your
>> committed this patch:
>>
>> /aci-gcc-fsf/sources/gcc-fsf/gccsrc/libstdc++-v3/testsuite/experimental/filesystem/operations/last_write_time.cc:127:
>> void test02(): Assertion 'last_write_time(f.path) == time' failed.
>> on arm*linux* and aarch64*linux* targets.
>
>
> That test will fail for targets where _GLIBCXX_USE_UTIMENSAT is not
> defined, as they use utime() instead which only supports second
> granularity.
>
> This should solve it, by only checking that the file times are within
> one second of the expected value.
>

Hi Jonathan,
Indeed your patch fixes the problem I reported.
Sorry for the delay, I was on holidays.

Thanks,

Christophe


>
> Tested x86_64-linux, committed to trunk.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Five patches for std::experimental::filesystem
  2016-11-02  9:09       ` Christophe Lyon
@ 2016-11-04 14:31         ` Christophe Lyon
       [not found]           ` <CAM8vyXesKD7BKjftkwbF6q7w8MhwOV-gZR_WCmW6i9OFjPbAJg@mail.gmail.com>
  0 siblings, 1 reply; 8+ messages in thread
From: Christophe Lyon @ 2016-11-04 14:31 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: libstdc++, gcc-patches

On 2 November 2016 at 10:09, Christophe Lyon <christophe.lyon@linaro.org> wrote:
> On 27 October 2016 at 15:34, Jonathan Wakely <jwakely@redhat.com> wrote:
>> On 26/10/16 09:24 +0200, Christophe Lyon wrote:
>>>
>>> Hi Jonathan,
>>>
>>> On 25 October 2016 at 17:32, Jonathan Wakely <jwakely@redhat.com> wrote:
>>>>
>>>> Two more fixes for the filesystem TS, and improved tests.
>>>>
>>>>   Handle negative times in filesystem::last_write_time
>>>>           * src/filesystem/ops.cc
>>>>        (last_write_time(const path&, file_time_type, error_code&)):
>>>> Handle
>>>>        negative times correctly.
>>>>        * testsuite/experimental/filesystem/operations/last_write_time.cc:
>>>>        Test writing file times.
>>>>
>>>>    Fix error handling in copy_file and equivalent
>>>>           * src/filesystem/ops.cc (do_copy_file): Report an error if
>>>> source
>>>> or
>>>>        destination is not a regular file (LWG 2712).
>>>>        (equivalent): Fix error handling and result when only one file
>>>> exists.
>>>>        * testsuite/experimental/filesystem/operations/copy.cc: Remove
>>>> files
>>>>        created by tests. Test copying directories.
>>>>        * testsuite/experimental/filesystem/operations/copy_file.cc:
>>>> Remove
>>>>        files created by tests.
>>>>        * testsuite/experimental/filesystem/operations/equivalent.cc: New.
>>>>        * testsuite/experimental/filesystem/operations/is_empty.cc: New.
>>>>        * testsuite/experimental/filesystem/operations/read_symlink.cc:
>>>> Remove
>>>>        file created by test.
>>>>        * testsuite/experimental/filesystem/operations/remove_all.cc: New.
>>>>        * testsuite/util/testsuite_fs.h (~scoped_file): Only try to remove
>>>>        file if path is non-empty, to support removal by other means.
>>>>
>>>> Tested x86_64-linux, committed to trunk.
>>>>
>>>>
>>> I can see failures in
>>> experimental/filesystem/operations/last_write_time.cc after your
>>> committed this patch:
>>>
>>> /aci-gcc-fsf/sources/gcc-fsf/gccsrc/libstdc++-v3/testsuite/experimental/filesystem/operations/last_write_time.cc:127:
>>> void test02(): Assertion 'last_write_time(f.path) == time' failed.
>>> on arm*linux* and aarch64*linux* targets.
>>
>>
>> That test will fail for targets where _GLIBCXX_USE_UTIMENSAT is not
>> defined, as they use utime() instead which only supports second
>> granularity.
>>
>> This should solve it, by only checking that the file times are within
>> one second of the expected value.
>>
>
> Hi Jonathan,
> Indeed your patch fixes the problem I reported.
> Sorry for the delay, I was on holidays.
>

.... but experimental/filesystem/iterators/recursive_directory_iterator.cc
now fails at execution on "old" arm targets (when forcing -march=armv5t).

Christophe


> Thanks,
>
> Christophe
>
>
>>
>> Tested x86_64-linux, committed to trunk.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Five patches for std::experimental::filesystem
       [not found]           ` <CAM8vyXesKD7BKjftkwbF6q7w8MhwOV-gZR_WCmW6i9OFjPbAJg@mail.gmail.com>
@ 2016-11-12 20:16             ` Jonathan Wakely
  0 siblings, 0 replies; 8+ messages in thread
From: Jonathan Wakely @ 2016-11-12 20:16 UTC (permalink / raw)
  To: Chris Fairles; +Cc: Christophe Lyon, libstdc++, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 183 bytes --]

On 10/11/16 17:29 +0000, Chris Fairles wrote:
>"cannot check is file is empty" -> *if

This patch fixes the typo. I haven't looked at the failure yet.

Will commit to trunk shortly.


[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 705 bytes --]

commit 9882c567342019c9561b44e611af9d42cdf23f3b
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Sat Nov 12 20:14:07 2016 +0000

    	* src/filesystem/ops.cc (is_empty): Fix typo in exception message.

diff --git a/libstdc++-v3/src/filesystem/ops.cc b/libstdc++-v3/src/filesystem/ops.cc
index 8ed0a10..0dcb1b4 100644
--- a/libstdc++-v3/src/filesystem/ops.cc
+++ b/libstdc++-v3/src/filesystem/ops.cc
@@ -1054,7 +1054,7 @@ fs::is_empty(const path& p)
   error_code ec;
   bool e = is_empty(p, ec);
   if (ec)
-    _GLIBCXX_THROW_OR_ABORT(filesystem_error("cannot check is file is empty",
+    _GLIBCXX_THROW_OR_ABORT(filesystem_error("cannot check if file is empty",
 					     p, ec));
   return e;
 }

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2016-11-12 20:16 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-24 16:50 [PATCH] Five patches for std::experimental::filesystem Jonathan Wakely
2016-10-25 15:32 ` Jonathan Wakely
2016-10-26  7:24   ` Christophe Lyon
2016-10-27 13:35     ` Jonathan Wakely
2016-11-02  9:09       ` Christophe Lyon
2016-11-04 14:31         ` Christophe Lyon
     [not found]           ` <CAM8vyXesKD7BKjftkwbF6q7w8MhwOV-gZR_WCmW6i9OFjPbAJg@mail.gmail.com>
2016-11-12 20:16             ` Jonathan Wakely
2016-10-26 13:34 ` 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).