diff options
author | redi <redi@138bc75d-0d04-0410-961f-82ee72b054a4> | 2016-04-19 18:02:39 +0000 |
---|---|---|
committer | redi <redi@138bc75d-0d04-0410-961f-82ee72b054a4> | 2016-04-19 18:02:39 +0000 |
commit | 00d7e7b3273148c277b44d6eec22cfd6f3900639 (patch) | |
tree | 969f95aefd9527a1f249ca7155f17f8ec69d9f1c | |
parent | 12133d9121316c5f197398700ec3826d0867954a (diff) | |
download | gcc-00d7e7b3273148c277b44d6eec22cfd6f3900639.tar.gz |
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().
git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@235215 138bc75d-0d04-0410-961f-82ee72b054a4
-rw-r--r-- | libstdc++-v3/ChangeLog | 9 | ||||
-rw-r--r-- | libstdc++-v3/src/filesystem/ops.cc | 63 | ||||
-rw-r--r-- | libstdc++-v3/testsuite/experimental/filesystem/operations/copy.cc | 120 |
3 files changed, 163 insertions, 29 deletions
diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog index 3562b25f3cc..1607e63d609 100644 --- a/libstdc++-v3/ChangeLog +++ b/libstdc++-v3/ChangeLog @@ -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. diff --git a/libstdc++-v3/src/filesystem/ops.cc b/libstdc++-v3/src/filesystem/ops.cc index 756e140d709..aa26cafa103 100644 --- a/libstdc++-v3/src/filesystem/ops.cc +++ b/libstdc++-v3/src/filesystem/ops.cc @@ -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. diff --git a/libstdc++-v3/testsuite/experimental/filesystem/operations/copy.cc b/libstdc++-v3/testsuite/experimental/filesystem/operations/copy.cc index 9e89002d58a..a5f6a3e8613 100644 --- a/libstdc++-v3/testsuite/experimental/filesystem/operations/copy.cc +++ b/libstdc++-v3/testsuite/experimental/filesystem/operations/copy.cc @@ -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(); } |