From 388058dd06b26e8f55d315b3c49b707d51c7a089 Mon Sep 17 00:00:00 2001 From: Jonathan Wakely Date: Fri, 5 Jan 2018 18:02:18 +0000 Subject: [PATCH] PR libstdc++/83626 handle ENOENT due to filesystem race PR libstdc++/83626 * src/filesystem/ops.cc (remove(const path&, error_code&)): Do not report an error for ENOENT. (remove_all(const path&)): Fix type of result variable. (remove_all(const path&, error_code&)): Use non-throwing increment for directory iterator. Call POSIX remove directly to avoid redundant calls to symlink_status. Do not report errors for ENOENT. * src/filesystem/std-ops.cc: Likewise. * testsuite/27_io/filesystem/operations/remove_all.cc: Test throwing overload. * testsuite/experimental/filesystem/operations/remove_all.cc: Likewise. From-SVN: r256283 --- libstdc++-v3/ChangeLog | 15 +++++++ libstdc++-v3/src/filesystem/ops.cc | 39 +++++++++++++------ libstdc++-v3/src/filesystem/std-ops.cc | 39 +++++++++++++------ .../27_io/filesystem/operations/remove_all.cc | 20 ++++++++++ .../filesystem/operations/remove_all.cc | 20 ++++++++++ 5 files changed, 111 insertions(+), 22 deletions(-) diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog index 82de1ff85d0..fb2eb63a693 100644 --- a/libstdc++-v3/ChangeLog +++ b/libstdc++-v3/ChangeLog @@ -1,3 +1,18 @@ +2018-01-05 Jonathan Wakely + + PR libstdc++/83626 + * src/filesystem/ops.cc (remove(const path&, error_code&)): Do not + report an error for ENOENT. + (remove_all(const path&)): Fix type of result variable. + (remove_all(const path&, error_code&)): Use non-throwing increment + for directory iterator. Call POSIX remove directly to avoid redundant + calls to symlink_status. Do not report errors for ENOENT. + * src/filesystem/std-ops.cc: Likewise. + * testsuite/27_io/filesystem/operations/remove_all.cc: Test throwing + overload. + * testsuite/experimental/filesystem/operations/remove_all.cc: + Likewise. + 2018-01-04 Jonathan Wakely PR libstdc++/83626 diff --git a/libstdc++-v3/src/filesystem/ops.cc b/libstdc++-v3/src/filesystem/ops.cc index 5a7cb599bb6..3690fb94d63 100644 --- a/libstdc++-v3/src/filesystem/ops.cc +++ b/libstdc++-v3/src/filesystem/ops.cc @@ -1025,13 +1025,16 @@ fs::remove(const path& p, error_code& ec) noexcept ec.clear(); return false; // Nothing to do, not an error. } - if (::remove(p.c_str()) != 0) + if (::remove(p.c_str()) == 0) { - ec.assign(errno, std::generic_category()); - return false; + ec.clear(); + return true; } - ec.clear(); - return true; + else if (errno == ENOENT) + ec.clear(); + else + ec.assign(errno, std::generic_category()); + return false; } @@ -1039,7 +1042,7 @@ std::uintmax_t fs::remove_all(const path& p) { error_code ec; - bool result = remove_all(p, ec); + const auto result = remove_all(p, ec); if (ec) _GLIBCXX_THROW_OR_ABORT(filesystem_error("cannot remove all", p, ec)); return result; @@ -1051,15 +1054,29 @@ fs::remove_all(const path& p, error_code& ec) noexcept const auto s = symlink_status(p, ec); if (!status_known(s)) return -1; + ec.clear(); + if (s.type() == file_type::not_found) + return 0; + uintmax_t count = 0; if (s.type() == file_type::directory) - for (directory_iterator d(p, ec), end; !ec && d != end; ++d) - count += fs::remove_all(d->path(), ec); - if (!ec && fs::remove(p, ec)) + { + for (directory_iterator d(p, ec), end; !ec && d != end; d.increment(ec)) + count += fs::remove_all(d->path(), ec); + if (ec.value() == ENOENT) + ec.clear(); + else if (ec) + return -1; + } + + if (::remove(p.c_str()) == 0) ++count; - if (ec) - return -1; + else if (errno != ENOENT) + { + ec.assign(errno, std::generic_category()); + return -1; + } return count; } diff --git a/libstdc++-v3/src/filesystem/std-ops.cc b/libstdc++-v3/src/filesystem/std-ops.cc index 6ff280a9c76..2411bbb7977 100644 --- a/libstdc++-v3/src/filesystem/std-ops.cc +++ b/libstdc++-v3/src/filesystem/std-ops.cc @@ -1274,13 +1274,16 @@ fs::remove(const path& p, error_code& ec) noexcept ec.clear(); return false; // Nothing to do, not an error. } - if (::remove(p.c_str()) != 0) + if (::remove(p.c_str()) == 0) { - ec.assign(errno, std::generic_category()); - return false; + ec.clear(); + return true; } - ec.clear(); - return true; + else if (errno == ENOENT) + ec.clear(); + else + ec.assign(errno, std::generic_category()); + return false; } @@ -1288,7 +1291,7 @@ std::uintmax_t fs::remove_all(const path& p) { error_code ec; - bool result = remove_all(p, ec); + const auto result = remove_all(p, ec); if (ec) _GLIBCXX_THROW_OR_ABORT(filesystem_error("cannot remove all", p, ec)); return result; @@ -1300,15 +1303,29 @@ fs::remove_all(const path& p, error_code& ec) const auto s = symlink_status(p, ec); if (!status_known(s)) return -1; + ec.clear(); + if (s.type() == file_type::not_found) + return 0; + uintmax_t count = 0; if (s.type() == file_type::directory) - for (directory_iterator d(p, ec), end; !ec && d != end; ++d) - count += fs::remove_all(d->path(), ec); - if (!ec && fs::remove(p, ec)) + { + for (directory_iterator d(p, ec), end; !ec && d != end; d.increment(ec)) + count += fs::remove_all(d->path(), ec); + if (ec.value() == ENOENT) + ec.clear(); + else if (ec) + return -1; + } + + if (::remove(p.c_str()) == 0) ++count; - if (ec) - return -1; + else if (errno != ENOENT) + { + ec.assign(errno, std::generic_category()); + return -1; + } return count; } diff --git a/libstdc++-v3/testsuite/27_io/filesystem/operations/remove_all.cc b/libstdc++-v3/testsuite/27_io/filesystem/operations/remove_all.cc index 8ee5c4d39c1..633cde57243 100644 --- a/libstdc++-v3/testsuite/27_io/filesystem/operations/remove_all.cc +++ b/libstdc++-v3/testsuite/27_io/filesystem/operations/remove_all.cc @@ -85,8 +85,28 @@ test01() b2.path.clear(); } +void +test02() +{ + const auto dir = __gnu_test::nonexistent_path(); + create_directories(dir/"a/b/c"); + std::uintmax_t n = remove_all(dir/"a"); + VERIFY( n == 3 ); + VERIFY( exists(dir) ); + VERIFY( !exists(dir/"a") ); + + n = remove_all(dir/"a"); + VERIFY( n == 0 ); + VERIFY( exists(dir) ); + + n = remove_all(dir); + VERIFY( n == 1 ); + VERIFY( !exists(dir) ); +} + int main() { test01(); + test02(); } diff --git a/libstdc++-v3/testsuite/experimental/filesystem/operations/remove_all.cc b/libstdc++-v3/testsuite/experimental/filesystem/operations/remove_all.cc index fa146b4317c..67f6e989d27 100644 --- a/libstdc++-v3/testsuite/experimental/filesystem/operations/remove_all.cc +++ b/libstdc++-v3/testsuite/experimental/filesystem/operations/remove_all.cc @@ -85,8 +85,28 @@ test01() b2.path.clear(); } +void +test02() +{ + const auto dir = __gnu_test::nonexistent_path(); + create_directories(dir/"a/b/c"); + std::uintmax_t n = remove_all(dir/"a"); + VERIFY( n == 3 ); + VERIFY( exists(dir) ); + VERIFY( !exists(dir/"a") ); + + n = remove_all(dir/"a"); + VERIFY( n == 0 ); + VERIFY( exists(dir) ); + + n = remove_all(dir); + VERIFY( n == 1 ); + VERIFY( !exists(dir) ); +} + int main() { test01(); + test02(); } -- 2.30.2