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
This commit is contained in:
Eric Fiselier
2018-02-04 02:43:32 +00:00
parent 7d251c57ac
commit af1fd7c75f
3 changed files with 28 additions and 13 deletions

View File

@@ -263,10 +263,13 @@ void __copy(const path& from, const path& to, copy_options options,
bool __copy_file(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) std::error_code *ec)
{ {
if (ec) ec->clear(); using StatT = struct ::stat;
if (ec)
ec->clear();
std::error_code m_ec; 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 is_regular_file(from_st)) {
if (not m_ec) if (not m_ec)
m_ec = make_error_code(errc::not_supported); m_ec = make_error_code(errc::not_supported);
@@ -274,7 +277,8 @@ bool __copy_file(const path& from, const path& to, copy_options options,
return false; 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)) { if (!status_known(to_st)) {
set_or_throw(m_ec, ec, "copy_file", from, to); set_or_throw(m_ec, ec, "copy_file", from, to);
return false; 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); set_or_throw(make_error_code(errc::not_supported), ec, "copy_file", from, to);
return false; 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)) { if (to_exists && bool(copy_options::skip_existing & options)) {
return false; return false;
} }
@@ -302,7 +311,8 @@ bool __copy_file(const path& from, const path& to, copy_options options,
return detail::copy_file_impl(from, to, from_st.permissions(), ec); return detail::copy_file_impl(from, to, from_st.permissions(), ec);
} }
else { else {
set_or_throw(make_error_code(errc::file_exists), ec, "copy", from, to); set_or_throw(make_error_code(errc::file_exists), ec, "copy_file", from,
to);
return false; return false;
} }
@@ -443,7 +453,7 @@ bool __equivalent(const path& p1, const path& p2, std::error_code *ec)
if (!exists(s2)) if (!exists(s2))
return make_unsupported_error(); return make_unsupported_error();
if (ec) ec->clear(); if (ec) ec->clear();
return (st1.st_dev == st2.st_dev && st1.st_ino == st2.st_ino); return detail::stat_equivalent(st1, st2);
} }

View File

@@ -70,17 +70,21 @@ TEST_CASE(test_error_reporting)
scoped_test_env env; scoped_test_env env;
const path file = env.create_file("file1", 42); const path file = env.create_file("file1", 42);
const path file2 = env.create_file("file2", 55); 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"); const path dne = env.make_env_path("dne");
{ // exists(to) && equivalent(to, from) { // exists(to) && equivalent(to, from)
std::error_code ec; 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_REQUIRE(ec);
TEST_CHECK(ec == std::make_error_code(std::errc::file_exists));
TEST_CHECK(checkThrow(file, file, ec)); TEST_CHECK(checkThrow(file, file, ec));
} }
{ // exists(to) && !(skip_existing | overwrite_existing | update_existing) { // exists(to) && !(skip_existing | overwrite_existing | update_existing)
std::error_code ec; std::error_code ec;
TEST_CHECK(fs::copy_file(file, file2, ec) == false); TEST_CHECK(fs::copy_file(file, file2, ec) == false);
TEST_REQUIRE(ec); TEST_REQUIRE(ec);
TEST_CHECK(ec == std::make_error_code(std::errc::file_exists));
TEST_CHECK(checkThrow(file, file2, ec)); 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_REQUIRE(fs::copy_file(file, fifo, copy_options::overwrite_existing, ec) == false);
TEST_CHECK(ec); TEST_CHECK(ec);
TEST_CHECK(ec != GetTestEC()); TEST_CHECK(ec != GetTestEC());
TEST_CHECK(ec == std::make_error_code(std::errc::not_supported));
TEST_CHECK(is_fifo(fifo)); TEST_CHECK(is_fifo(fifo));
} }
} }

View File

@@ -63,7 +63,7 @@
<tr><td><a href="https://wg21.link/LWG2243">2243</a></td><td><tt>istream::putback</tt> problem</td><td>Jacksonville</td><td>Complete</td></tr> <tr><td><a href="https://wg21.link/LWG2243">2243</a></td><td><tt>istream::putback</tt> problem</td><td>Jacksonville</td><td>Complete</td></tr>
<tr><td><a href="https://wg21.link/LWG2816">2816</a></td><td><tt>resize_file</tt> has impossible postcondition</td><td>Jacksonville</td><td><i>Nothing to do</i></td></tr> <tr><td><a href="https://wg21.link/LWG2816">2816</a></td><td><tt>resize_file</tt> has impossible postcondition</td><td>Jacksonville</td><td><i>Nothing to do</i></td></tr>
<tr><td><a href="https://wg21.link/LWG2843">2843</a></td><td>Unclear behavior of <tt>std::pmr::memory_resource::do_allocate()</tt></td><td>Jacksonville</td><td></td></tr> <tr><td><a href="https://wg21.link/LWG2843">2843</a></td><td>Unclear behavior of <tt>std::pmr::memory_resource::do_allocate()</tt></td><td>Jacksonville</td><td></td></tr>
<tr><td><a href="https://wg21.link/LWG2849">2849</a></td><td>Why does <tt>!is_regular_file(from)</tt> cause <tt>copy_file</tt> to report a "file already exists" error?</td><td>Jacksonville</td><td></td></tr> <tr><td><a href="https://wg21.link/LWG2849">2849</a></td><td>Why does <tt>!is_regular_file(from)</tt> cause <tt>copy_file</tt> to report a "file already exists" error?</td><td>Jacksonville</td><td>Nothing to do</td></tr>
<tr><td><a href="https://wg21.link/LWG2851">2851</a></td><td><tt>std::filesystem</tt> enum classes are now underspecified</td><td>Jacksonville</td><td></td></tr> <tr><td><a href="https://wg21.link/LWG2851">2851</a></td><td><tt>std::filesystem</tt> enum classes are now underspecified</td><td>Jacksonville</td><td></td></tr>
<tr><td><a href="https://wg21.link/LWG2969">2969</a></td><td><tt>polymorphic_allocator::construct()</tt> shouldn't pass <tt>resource()</tt></td><td>Jacksonville</td><td></td></tr> <tr><td><a href="https://wg21.link/LWG2969">2969</a></td><td><tt>polymorphic_allocator::construct()</tt> shouldn't pass <tt>resource()</tt></td><td>Jacksonville</td><td></td></tr>
<tr><td><a href="https://wg21.link/LWG2975">2975</a></td><td>Missing case for <tt>pair</tt> construction in scoped and polymorphic allocators</td><td>Jacksonville</td><td></td></tr> <tr><td><a href="https://wg21.link/LWG2975">2975</a></td><td>Missing case for <tt>pair</tt> construction in scoped and polymorphic allocators</td><td>Jacksonville</td><td></td></tr>
@@ -100,7 +100,7 @@
<li> 2243 - We already do this.</li> <li> 2243 - We already do this.</li>
<li> 2816 - Wording changes only</li> <li> 2816 - Wording changes only</li>
<li> 2843 - We don't have PMRs yet</li> <li> 2843 - We don't have PMRs yet</li>
<li> 2849 - Eric? </li> <li> 2849 - We already report different errors. </li>
<li> 2851 - Eric? </li> <li> 2851 - Eric? </li>
<li> 2969 - We don't have PMRs yet</li> <li> 2969 - We don't have PMRs yet</li>
<li> 2975 - We can do the scoped_ bit, but the PMR stuff will have to wait.</li> <li> 2975 - We can do the scoped_ bit, but the PMR stuff will have to wait.</li>