libstdc++/70609 fix filesystem::copy()
authorJonathan Wakely <jwakely@redhat.com>
Tue, 19 Apr 2016 18:02:39 +0000 (19:02 +0100)
committerJonathan Wakely <redi@gcc.gnu.org>
Tue, 19 Apr 2016 18:02:39 +0000 (19:02 +0100)
PR libstdc++/70609
* src/filesystem/ops.cc (close_fd): New function.
(do_copy_file): Set permissions before copying file contents. Check
result of closing file descriptors. Don't copy streambuf when file
is empty.
(copy(const path&, const path&, copy_options, error_code&)): Use
lstat for source file when copy_symlinks is set.
* testsuite/experimental/filesystem/operations/copy.cc: Test copy().

From-SVN: r235215

libstdc++-v3/ChangeLog
libstdc++-v3/src/filesystem/ops.cc
libstdc++-v3/testsuite/experimental/filesystem/operations/copy.cc

index 3562b25f3ccc8fab9ae1f2746928f6e61ddb0df5..1607e63d6093094dc15a1a5f37a6f135dc3e6c5c 100644 (file)
@@ -1,5 +1,14 @@
 2016-04-19  Jonathan Wakely  <jwakely@redhat.com>
 
+       PR libstdc++/70609
+       * src/filesystem/ops.cc (close_fd): New function.
+       (do_copy_file): Set permissions before copying file contents. Check
+       result of closing file descriptors. Don't copy streambuf when file
+       is empty.
+       (copy(const path&, const path&, copy_options, error_code&)): Use
+       lstat for source file when copy_symlinks is set.
+       * testsuite/experimental/filesystem/operations/copy.cc: Test copy().
+
        * include/experimental/bits/fs_fwd.h (operator&, operator|, operator^,
        operator~ operator&=, operator|=, operator^=): Add noexcept to
        overloaded operators for copy_options, perms and directory_options.
index 756e140d709767e3d52be4eff0acaf738740a2ee..aa26cafa1032657e79861a235f4cbbf5c7e48071 100644 (file)
@@ -300,6 +300,17 @@ namespace
     };
   }
 
+  // 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,
@@ -376,7 +387,8 @@ namespace
       }
 
     struct CloseFD {
-      ~CloseFD() { if (fd != -1) ::close(fd); }
+      ~CloseFD() { if (fd != -1) close_fd(fd); }
+      bool close() { return close_fd(std::exchange(fd, -1)); }
       int fd;
     };
 
@@ -401,34 +413,49 @@ namespace
        return false;
       }
 
+#ifdef _GLIBCXX_USE_FCHMOD
+    if (::fchmod(out.fd, from_st->st_mode))
+#elif _GLIBCXX_USE_FCHMODAT
+    if (::fchmodat(AT_FDCWD, to.c_str(), from_st->st_mode, 0))
+#else
+    if (::chmod(to.c_str(), from_st->st_mode))
+#endif
+      {
+       ec.assign(errno, std::generic_category());
+       return false;
+      }
+
 #ifdef _GLIBCXX_USE_SENDFILE
-    auto n = ::sendfile(out.fd, in.fd, nullptr, from_st->st_size);
+    const auto n = ::sendfile(out.fd, in.fd, nullptr, from_st->st_size);
     if (n != from_st->st_size)
       {
        ec.assign(errno, std::generic_category());
        return false;
       }
+    if (!out.close() || !in.close())
+      {
+       ec.assign(errno, std::generic_category());
+       return false;
+      }
 #else
     __gnu_cxx::stdio_filebuf<char> sbin(in.fd, std::ios::in);
     __gnu_cxx::stdio_filebuf<char> sbout(out.fd, std::ios::out);
-    if ( !(std::ostream(&sbout) << &sbin) )
+    if (sbin.is_open())
+      in.fd = -1;
+    if (sbout.is_open())
+      out.fd = -1;
+    if (from_st->st_size && !(std::ostream(&sbout) << &sbin))
       {
        ec = std::make_error_code(std::errc::io_error);
        return false;
       }
-#endif
-
-#ifdef _GLIBCXX_USE_FCHMOD
-    if (::fchmod(out.fd, from_st->st_mode))
-#elif _GLIBCXX_USE_FCHMODAT
-    if (::fchmodat(AT_FDCWD, to.c_str(), from_st->st_mode, 0))
-#else
-    if (::chmod(to.c_str(), from_st->st_mode))
-#endif
+    if (sbout.close() || sbin.close())
       {
        ec.assign(errno, std::generic_category());
        return false;
       }
+#endif
+
     ec.clear();
     return true;
   }
@@ -439,13 +466,15 @@ void
 fs::copy(const path& from, const path& to, copy_options options,
         error_code& ec) noexcept
 {
-  bool skip_symlinks = is_set(options, copy_options::skip_symlinks);
-  bool create_symlinks = is_set(options, copy_options::create_symlinks);
-  bool use_lstat = create_symlinks || skip_symlinks;
+  const bool skip_symlinks = is_set(options, copy_options::skip_symlinks);
+  const bool create_symlinks = is_set(options, copy_options::create_symlinks);
+  const bool copy_symlinks = is_set(options, copy_options::copy_symlinks);
+  const bool use_lstat = create_symlinks || skip_symlinks;
 
   file_status f, t;
   stat_type from_st, to_st;
-  if (use_lstat
+  // N4099 doesn't check copy_symlinks here, but I think that's a defect.
+  if (use_lstat || copy_symlinks
       ? ::lstat(from.c_str(), &from_st)
       : ::stat(from.c_str(), &from_st))
     {
@@ -488,7 +517,7 @@ fs::copy(const path& from, const path& to, copy_options options,
     {
       if (skip_symlinks)
        ec.clear();
-      else if (!exists(t) && is_set(options, copy_options::copy_symlinks))
+      else if (!exists(t) && copy_symlinks)
        copy_symlink(from, to, ec);
       else
        // Not clear what should be done here.
index 9e89002d58a4a750382531458513d0de9ea93869..a5f6a3e861371dab66156b4ff6f681a63f663493 100644 (file)
 // 15.3 Copy [fs.op.copy]
 
 #include <experimental/filesystem>
+#include <fstream>
 #include <testsuite_fs.h>
 #include <testsuite_hooks.h>
 
-using std::experimental::filesystem::path;
+namespace fs = std::experimental::filesystem;
 
+// Test error conditions.
 void
 test01()
 {
   bool test __attribute__((unused)) = false;
 
-  for (const path& p : __gnu_test::test_paths)
-    VERIFY( absolute(p).is_absolute() );
+  auto p = __gnu_test::nonexistent_path();
+  std::error_code ec;
+
+  VERIFY( !fs::exists(p) );
+  fs::copy(p, ".", fs::copy_options::none, ec);
+  VERIFY( ec );
+
+  ec.clear();
+  fs::copy(".", ".", fs::copy_options::none, ec);
+  VERIFY( ec );
+
+  std::ofstream{p.native()};
+  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);
 }
 
+// Test is_symlink(f) case.
 void
 test02()
 {
   bool test __attribute__((unused)) = false;
 
-  path p1("/");
-  VERIFY( absolute(p1) == p1 );
-  VERIFY( absolute(p1, "/bar") == p1 );
-  path p2("/foo");
-  VERIFY( absolute(p2) == p2 );
-  VERIFY( absolute(p2, "/bar") == p2 );
-  path p3("foo");
-  VERIFY( absolute(p3) != p3 );
-  VERIFY( absolute(p3, "/bar") == "/bar/foo" );
+  auto from = __gnu_test::nonexistent_path();
+  auto to = __gnu_test::nonexistent_path();
+  std::error_code ec;
+
+  fs::create_symlink(".", from, ec);
+  VERIFY( !ec );
+  VERIFY( fs::exists(from) );
+
+  fs::copy(from, to, fs::copy_options::skip_symlinks, ec);
+  VERIFY( !ec );
+  VERIFY( !fs::exists(to) );
+
+  fs::copy(from, to, fs::copy_options::skip_symlinks, ec);
+  VERIFY( !ec );
+  VERIFY( !fs::exists(to) );
+
+  fs::copy(from, to,
+           fs::copy_options::skip_symlinks|fs::copy_options::copy_symlinks,
+           ec);
+  VERIFY( !ec );
+  VERIFY( !fs::exists(to) );
+
+  fs::copy(from, to, fs::copy_options::copy_symlinks, ec);
+  VERIFY( !ec );
+  VERIFY( fs::exists(to) );
+
+  fs::copy(from, to, fs::copy_options::copy_symlinks, ec);
+  VERIFY( ec );
+
+  remove(from, ec);
+  remove(to, ec);
+}
+
+// Test is_regular_file(f) case.
+void
+test03()
+{
+  bool test __attribute__((unused)) = false;
+
+  auto from = __gnu_test::nonexistent_path();
+  auto to = __gnu_test::nonexistent_path();
+
+  // test empty file
+  std::ofstream{from.native()};
+  VERIFY( fs::exists(from) );
+  VERIFY( fs::file_size(from) == 0 );
+  fs::copy(from, to);
+  VERIFY( fs::exists(to) );
+  VERIFY( fs::file_size(to) == 0 );
+
+  remove(to);
+  VERIFY( !fs::exists(to) );
+  std::ofstream{from.native()} << "Hello, filesystem!";
+  VERIFY( fs::file_size(from) != 0 );
+  fs::copy(from, to);
+  VERIFY( fs::exists(to) );
+  VERIFY( fs::file_size(to) == fs::file_size(from) );
+}
+
+// Test is_directory(f) case.
+void
+test04()
+{
+  bool test __attribute__((unused)) = false;
+
+  auto from = __gnu_test::nonexistent_path();
+  auto to = __gnu_test::nonexistent_path();
+  std::error_code ec;
+
+}
+
+// Test no-op cases.
+void
+test05()
+{
+  bool test __attribute__((unused)) = false;
+
+  auto to = __gnu_test::nonexistent_path();
+  std::error_code ec;
+
+  fs::copy("/", to, fs::copy_options::create_symlinks, ec);
+  VERIFY( !ec );
 }
 
 int
@@ -56,4 +149,7 @@ main()
 {
   test01();
   test02();
+  test03();
+  test04();
+  test05();
 }