commit 461aaa6fb392aad7fc0e2c210b0b493b9335b38c
Author: Jonathan Wakely <jwakely@redhat.com>
Date: Tue Apr 19 18:32:03 2016 +0100
libstdc++/70609 fix filesystem::copy()
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().
@@ -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,23 +413,6 @@ namespace
return false;
}
-#ifdef _GLIBCXX_USE_SENDFILE
- 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;
- }
-#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) )
- {
- 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
@@ -429,6 +424,38 @@ namespace
ec.assign(errno, std::generic_category());
return false;
}
+
+#ifdef _GLIBCXX_USE_SENDFILE
+ 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 (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;
+ }
+ 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.
@@ -21,34 +21,127 @@
// 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();
}