From c312b30abb845d514d7ab0a0a1a9745925a2263a Mon Sep 17 00:00:00 2001 From: Rodrigo Salazar <4rodrigosalazar@gmail.com> Date: Wed, 10 Apr 2024 17:46:39 -0700 Subject: [PATCH 1/2] [libcxx] Correct and clean-up filesystem:: error_code paths --- libcxx/src/filesystem/operations.cpp | 28 ++++++++++--------- .../weakly_canonical.pass.cpp | 8 ++++++ 2 files changed, 23 insertions(+), 13 deletions(-) diff --git a/libcxx/src/filesystem/operations.cpp b/libcxx/src/filesystem/operations.cpp index 62bb248d5eca9..6a8e42d2090c4 100644 --- a/libcxx/src/filesystem/operations.cpp +++ b/libcxx/src/filesystem/operations.cpp @@ -126,9 +126,6 @@ void __copy(const path& from, const path& to, copy_options options, error_code* return err.report(errc::function_not_supported); } - if (ec) - ec->clear(); - if (is_symlink(f)) { if (bool(copy_options::skip_symlinks & options)) { // do nothing @@ -166,15 +163,15 @@ void __copy(const path& from, const path& to, copy_options options, error_code* return; } error_code m_ec2; - for (; it != directory_iterator(); it.increment(m_ec2)) { - if (m_ec2) { - return err.report(m_ec2); - } + for (; !m_ec2 && it != directory_iterator(); it.increment(m_ec2)) { __copy(it->path(), to / it->path().filename(), options | copy_options::__in_recursive_copy, ec); if (ec && *ec) { return; } } + if (m_ec2) { + return err.report(m_ec2); + } } } @@ -936,23 +933,28 @@ path __weakly_canonical(const path& p, error_code* ec) { --PP; vector DNEParts; + error_code m_ec; while (PP.State != PathParser::PS_BeforeBegin) { tmp.assign(createView(p.native().data(), &PP.RawEntry.back())); - error_code m_ec; file_status st = __status(tmp, &m_ec); if (!status_known(st)) { return err.report(m_ec); } else if (exists(st)) { - result = __canonical(tmp, ec); + result = __canonical(tmp, &m_ec); + if (m_ec) { + return err.report(m_ec); + } break; } DNEParts.push_back(*PP); --PP; } - if (PP.State == PathParser::PS_BeforeBegin) - result = __canonical("", ec); - if (ec) - ec->clear(); + if (PP.State == PathParser::PS_BeforeBegin) { + result = __canonical("", &m_ec); + if (m_ec) { + return err.report(m_ec); + } + } if (DNEParts.empty()) return result; for (auto It = DNEParts.rbegin(); It != DNEParts.rend(); ++It) diff --git a/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.weakly_canonical/weakly_canonical.pass.cpp b/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.weakly_canonical/weakly_canonical.pass.cpp index 053bddcad9b43..4e4f459f8cf36 100644 --- a/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.weakly_canonical/weakly_canonical.pass.cpp +++ b/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.weakly_canonical/weakly_canonical.pass.cpp @@ -79,6 +79,14 @@ int main(int, char**) { TEST_REQUIRE(PathEq(output, expect), TEST_WRITE_CONCATENATED( "Input: ", TC.input.string(), "\nExpected: ", expect.string(), "\nOutput: ", output.string())); + + // Get coverage over the error_code form of the api. + std::error_code ec; + const fs::path output_c = fs::weakly_canonical(p, ec); + + TEST_REQUIRE(PathEq(output_c, expect), + TEST_WRITE_CONCATENATED( + "Input: ", TC.input.string(), "\nExpected: ", expect.string(), "\nOutput: ", output_c.string())); } return 0; } From 72246a74c40b3610f068ac4a073ec694c04bdf54 Mon Sep 17 00:00:00 2001 From: Louis Dionne Date: Wed, 12 Jun 2024 12:10:42 -0400 Subject: [PATCH 2/2] Minor reformatting in the test --- .../weakly_canonical.pass.cpp | 24 +++++++++++-------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.weakly_canonical/weakly_canonical.pass.cpp b/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.weakly_canonical/weakly_canonical.pass.cpp index 4e4f459f8cf36..59bb3c78667b4 100644 --- a/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.weakly_canonical/weakly_canonical.pass.cpp +++ b/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.weakly_canonical/weakly_canonical.pass.cpp @@ -74,19 +74,23 @@ int main(int, char**) { fs::path p = TC.input; fs::path expect = TC.expect; expect.make_preferred(); - const fs::path output = fs::weakly_canonical(p); - TEST_REQUIRE(PathEq(output, expect), - TEST_WRITE_CONCATENATED( - "Input: ", TC.input.string(), "\nExpected: ", expect.string(), "\nOutput: ", output.string())); + { + const fs::path output = fs::weakly_canonical(p); + TEST_REQUIRE(PathEq(output, expect), + TEST_WRITE_CONCATENATED( + "Input: ", TC.input.string(), "\nExpected: ", expect.string(), "\nOutput: ", output.string())); + } - // Get coverage over the error_code form of the api. - std::error_code ec; - const fs::path output_c = fs::weakly_canonical(p, ec); + // Test the error_code variant + { + std::error_code ec; + const fs::path output_c = fs::weakly_canonical(p, ec); - TEST_REQUIRE(PathEq(output_c, expect), - TEST_WRITE_CONCATENATED( - "Input: ", TC.input.string(), "\nExpected: ", expect.string(), "\nOutput: ", output_c.string())); + TEST_REQUIRE(PathEq(output_c, expect), + TEST_WRITE_CONCATENATED( + "Input: ", TC.input.string(), "\nExpected: ", expect.string(), "\nOutput: ", output_c.string())); + } } return 0; }