[gcc(refs/vendors/redhat/heads/gcc-9-branch)] libstdc++: Fix error handling in filesystem::remove_all (PR93201)
Jakub Jelinek
jakub@gcc.gnu.org
Thu Jan 23 10:16:00 GMT 2020
https://gcc.gnu.org/g:58277c62ad3db1846ef6210c486609805ddd9878
commit 58277c62ad3db1846ef6210c486609805ddd9878
Author: Jonathan Wakely <jwakely@redhat.com>
Date: Wed Jan 8 21:48:23 2020 +0000
libstdc++: Fix error handling in filesystem::remove_all (PR93201)
When recursing into a directory, any errors that occur while removing a
directory entry are ignored, because the subsequent increment of the
directory iterator clears the error_code object.
This fixes that bug by checking the result of each recursive operation
before incrementing. This is a change in observable behaviour, because
previously other directory entries would still be removed even if one
(or more) couldn't be removed due to errors. Now the operation stops on
the first error, which is what the code intended to do all along. The
standard doesn't specify what happens in this case (because the order
that the entries are processed is unspecified anyway).
PR libstdc++/93201
* src/c++17/fs_ops.cc (remove_all(const path&, error_code&)): Check
result of recursive call before incrementing iterator.
* src/filesystem/ops.cc (remove_all(const path&, error_code&)):
Likewise.
* testsuite/27_io/filesystem/operations/remove_all.cc: Check errors
are reported correctly.
* testsuite/experimental/filesystem/operations/remove_all.cc: Likewise.
Diff:
---
libstdc++-v3/ChangeLog | 11 ++++++++
libstdc++-v3/src/c++17/fs_ops.cc | 17 +++++++----
libstdc++-v3/src/filesystem/ops.cc | 17 +++++++----
.../27_io/filesystem/operations/remove_all.cc | 33 ++++++++++++++++++++++
.../filesystem/operations/remove_all.cc | 33 ++++++++++++++++++++++
5 files changed, 99 insertions(+), 12 deletions(-)
diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog
index 3b5f453..0180d77 100644
--- a/libstdc++-v3/ChangeLog
+++ b/libstdc++-v3/ChangeLog
@@ -1,3 +1,14 @@
+2020-01-08 Jonathan Wakely <jwakely@redhat.com>
+
+ PR libstdc++/93201
+ * src/c++17/fs_ops.cc (remove_all(const path&, error_code&)): Check
+ result of recursive call before incrementing iterator.
+ * src/filesystem/ops.cc (remove_all(const path&, error_code&)):
+ Likewise.
+ * testsuite/27_io/filesystem/operations/remove_all.cc: Check errors
+ are reported correctly.
+ * testsuite/experimental/filesystem/operations/remove_all.cc: Likewise.
+
2019-12-11 Jonathan Wakely <jwakely@redhat.com>
Backport from mainline
diff --git a/libstdc++-v3/src/c++17/fs_ops.cc b/libstdc++-v3/src/c++17/fs_ops.cc
index d806481..d918c2a 100644
--- a/libstdc++-v3/src/c++17/fs_ops.cc
+++ b/libstdc++-v3/src/c++17/fs_ops.cc
@@ -1299,12 +1299,17 @@ fs::remove_all(const path& p, error_code& ec)
uintmax_t count = 0;
if (s.type() == file_type::directory)
{
- 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;
+ directory_iterator d(p, ec), end;
+ while (!ec && d != end)
+ {
+ const auto removed = fs::remove_all(d->path(), ec);
+ if (removed == numeric_limits<uintmax_t>::max())
+ return -1;
+ count += removed;
+ d.increment(ec);
+ if (ec)
+ return -1;
+ }
}
if (fs::remove(p, ec))
diff --git a/libstdc++-v3/src/filesystem/ops.cc b/libstdc++-v3/src/filesystem/ops.cc
index 36b5d2c..a5887f3 100644
--- a/libstdc++-v3/src/filesystem/ops.cc
+++ b/libstdc++-v3/src/filesystem/ops.cc
@@ -1098,12 +1098,17 @@ fs::remove_all(const path& p, error_code& ec) noexcept
uintmax_t count = 0;
if (s.type() == file_type::directory)
{
- 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;
+ directory_iterator d(p, ec), end;
+ while (!ec && d != end)
+ {
+ const auto removed = fs::remove_all(d->path(), ec);
+ if (removed == numeric_limits<uintmax_t>::max())
+ return -1;
+ count += removed;
+ d.increment(ec);
+ if (ec)
+ return -1;
+ }
}
if (fs::remove(p, ec))
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 a19bac9..b0b176f 100644
--- a/libstdc++-v3/testsuite/27_io/filesystem/operations/remove_all.cc
+++ b/libstdc++-v3/testsuite/27_io/filesystem/operations/remove_all.cc
@@ -140,10 +140,43 @@ test03()
VERIFY( !exists(p) );
}
+void
+test04()
+{
+#if defined(__MINGW32__) || defined(__MINGW64__)
+ // no permissions
+#else
+ // PR libstdc++/93201
+ std::error_code ec;
+ std::uintmax_t n;
+
+ auto dir = __gnu_test::nonexistent_path();
+ fs::create_directory(dir);
+ __gnu_test::scoped_file f(dir/"file");
+ // remove write permission on the directory:
+ fs::permissions(dir, fs::perms::owner_read|fs::perms::owner_exec);
+ n = fs::remove_all(dir, ec);
+ VERIFY( n == -1 );
+ VERIFY( ec == std::errc::permission_denied ); // not ENOTEMPTY
+
+ try {
+ fs::remove_all(dir);
+ VERIFY( false );
+ } catch (const fs::filesystem_error& e) {
+ VERIFY( e.code() == std::errc::permission_denied );
+ // First path is the argument to remove_all
+ VERIFY( e.path1() == dir );
+ }
+
+ fs::permissions(dir, fs::perms::owner_write, fs::perm_options::add);
+#endif
+}
+
int
main()
{
test01();
test02();
test03();
+ test04();
}
diff --git a/libstdc++-v3/testsuite/experimental/filesystem/operations/remove_all.cc b/libstdc++-v3/testsuite/experimental/filesystem/operations/remove_all.cc
index 99fb14a..9d51a66 100644
--- a/libstdc++-v3/testsuite/experimental/filesystem/operations/remove_all.cc
+++ b/libstdc++-v3/testsuite/experimental/filesystem/operations/remove_all.cc
@@ -108,9 +108,42 @@ test02()
VERIFY( !exists(dir) );
}
+void
+test04()
+{
+#if defined(__MINGW32__) || defined(__MINGW64__)
+ // no permissions
+#else
+ // PR libstdc++/93201
+ std::error_code ec;
+ std::uintmax_t n;
+
+ auto dir = __gnu_test::nonexistent_path();
+ fs::create_directory(dir);
+ __gnu_test::scoped_file f(dir/"file");
+ // remove write permission on the directory:
+ fs::permissions(dir, fs::perms::owner_read|fs::perms::owner_exec);
+ n = fs::remove_all(dir, ec);
+ VERIFY( n == -1 );
+ VERIFY( ec == std::errc::permission_denied ); // not ENOTEMPTY
+
+ try {
+ fs::remove_all(dir);
+ VERIFY( false );
+ } catch (const fs::filesystem_error& e) {
+ VERIFY( e.code() == std::errc::permission_denied );
+ // First path is the argument to remove_all
+ VERIFY( e.path1() == dir );
+ }
+
+ fs::permissions(dir, fs::perms::owner_write|fs::perms::add_perms);
+#endif
+}
+
int
main()
{
test01();
test02();
+ test04();
}
More information about the Libstdc++-cvs
mailing list