From af1fd7c75f5cb2a19c11339d60b4fa2da31fd901 Mon Sep 17 00:00:00 2001 From: Eric Fiselier Date: Sun, 4 Feb 2018 02:43:32 +0000 Subject: [PATCH] Address LWG 2849 and fix missing failure condition in copy_file. Previously copy_file didn't handle the case where the input and output were the same file. git-svn-id: https://llvm.org/svn/llvm-project/libcxx/trunk@324187 91177308-0d34-0410-b5e6-96231b3b80d8 --- src/experimental/filesystem/operations.cpp | 30 ++++++++++++------- .../fs.op.copy_file/copy_file.pass.cpp | 7 ++++- www/upcoming_meeting.html | 4 +-- 3 files changed, 28 insertions(+), 13 deletions(-) diff --git a/src/experimental/filesystem/operations.cpp b/src/experimental/filesystem/operations.cpp index 91aee6ffb..2bc28c21d 100644 --- a/src/experimental/filesystem/operations.cpp +++ b/src/experimental/filesystem/operations.cpp @@ -263,18 +263,22 @@ void __copy(const path& from, const path& to, copy_options options, bool __copy_file(const path& from, const path& to, copy_options options, std::error_code *ec) { - if (ec) ec->clear(); + using StatT = struct ::stat; + if (ec) + ec->clear(); std::error_code m_ec; - auto from_st = detail::posix_stat(from, &m_ec); + StatT from_stat; + auto from_st = detail::posix_stat(from, from_stat, &m_ec); if (not is_regular_file(from_st)) { - if (not m_ec) - m_ec = make_error_code(errc::not_supported); - set_or_throw(m_ec, ec, "copy_file", from, to); - return false; + if (not m_ec) + m_ec = make_error_code(errc::not_supported); + set_or_throw(m_ec, ec, "copy_file", from, to); + return false; } - auto to_st = detail::posix_stat(to, &m_ec); + StatT to_stat; + auto to_st = detail::posix_stat(to, to_stat, &m_ec); if (!status_known(to_st)) { set_or_throw(m_ec, ec, "copy_file", from, to); return false; @@ -285,6 +289,11 @@ bool __copy_file(const path& from, const path& to, copy_options options, set_or_throw(make_error_code(errc::not_supported), ec, "copy_file", from, to); return false; } + if (to_exists && detail::stat_equivalent(from_stat, to_stat)) { + set_or_throw(make_error_code(errc::file_exists), ec, "copy_file", from, + to); + return false; + } if (to_exists && bool(copy_options::skip_existing & options)) { return false; } @@ -302,8 +311,9 @@ bool __copy_file(const path& from, const path& to, copy_options options, return detail::copy_file_impl(from, to, from_st.permissions(), ec); } else { - set_or_throw(make_error_code(errc::file_exists), ec, "copy", from, to); - return false; + set_or_throw(make_error_code(errc::file_exists), ec, "copy_file", from, + to); + return false; } _LIBCPP_UNREACHABLE(); @@ -443,7 +453,7 @@ bool __equivalent(const path& p1, const path& p2, std::error_code *ec) if (!exists(s2)) return make_unsupported_error(); if (ec) ec->clear(); - return (st1.st_dev == st2.st_dev && st1.st_ino == st2.st_ino); + return detail::stat_equivalent(st1, st2); } diff --git a/test/std/experimental/filesystem/fs.op.funcs/fs.op.copy_file/copy_file.pass.cpp b/test/std/experimental/filesystem/fs.op.funcs/fs.op.copy_file/copy_file.pass.cpp index 8c5241c71..ee2ad1c46 100644 --- a/test/std/experimental/filesystem/fs.op.funcs/fs.op.copy_file/copy_file.pass.cpp +++ b/test/std/experimental/filesystem/fs.op.funcs/fs.op.copy_file/copy_file.pass.cpp @@ -70,17 +70,21 @@ TEST_CASE(test_error_reporting) scoped_test_env env; const path file = env.create_file("file1", 42); const path file2 = env.create_file("file2", 55); + const path non_regular_file = env.create_fifo("non_reg"); const path dne = env.make_env_path("dne"); { // exists(to) && equivalent(to, from) std::error_code ec; - TEST_CHECK(fs::copy_file(file, file, ec) == false); + TEST_CHECK(fs::copy_file(file, file, copy_options::overwrite_existing, + ec) == false); TEST_REQUIRE(ec); + TEST_CHECK(ec == std::make_error_code(std::errc::file_exists)); TEST_CHECK(checkThrow(file, file, ec)); } { // exists(to) && !(skip_existing | overwrite_existing | update_existing) std::error_code ec; TEST_CHECK(fs::copy_file(file, file2, ec) == false); TEST_REQUIRE(ec); + TEST_CHECK(ec == std::make_error_code(std::errc::file_exists)); TEST_CHECK(checkThrow(file, file2, ec)); } } @@ -181,6 +185,7 @@ TEST_CASE(non_regular_file_test) TEST_REQUIRE(fs::copy_file(file, fifo, copy_options::overwrite_existing, ec) == false); TEST_CHECK(ec); TEST_CHECK(ec != GetTestEC()); + TEST_CHECK(ec == std::make_error_code(std::errc::not_supported)); TEST_CHECK(is_fifo(fifo)); } } diff --git a/www/upcoming_meeting.html b/www/upcoming_meeting.html index f5b2e91fd..720552357 100644 --- a/www/upcoming_meeting.html +++ b/www/upcoming_meeting.html @@ -63,7 +63,7 @@ 2243istream::putback problemJacksonvilleComplete 2816resize_file has impossible postconditionJacksonvilleNothing to do 2843Unclear behavior of std::pmr::memory_resource::do_allocate()Jacksonville -2849Why does !is_regular_file(from) cause copy_file to report a "file already exists" error?Jacksonville +2849Why does !is_regular_file(from) cause copy_file to report a "file already exists" error?JacksonvilleNothing to do 2851std::filesystem enum classes are now underspecifiedJacksonville 2969polymorphic_allocator::construct() shouldn't pass resource()Jacksonville 2975Missing case for pair construction in scoped and polymorphic allocatorsJacksonville @@ -100,7 +100,7 @@
  • 2243 - We already do this.
  • 2816 - Wording changes only
  • 2843 - We don't have PMRs yet
    • -
  • 2849 - Eric?
    • +
  • 2849 - We already report different errors.
  • 2851 - Eric?
  • 2969 - We don't have PMRs yet
  • 2975 - We can do the scoped_ bit, but the PMR stuff will have to wait.