From: Jonathan Wakely Date: Wed, 8 Jan 2020 21:48:23 +0000 (+0000) Subject: libstdc++: Fix error handling in filesystem::remove_all (PR93201) X-Git-Tag: releases/gcc-9.3.0~245 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=97173f7e2d95e65538fe946e3c64b0ccf4529239;p=thirdparty%2Fgcc.git 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. From-SVN: r280020 --- diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog index 3b5f453b04d6..0180d7737cb6 100644 --- a/libstdc++-v3/ChangeLog +++ b/libstdc++-v3/ChangeLog @@ -1,3 +1,14 @@ +2020-01-08 Jonathan Wakely + + 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 Backport from mainline diff --git a/libstdc++-v3/src/c++17/fs_ops.cc b/libstdc++-v3/src/c++17/fs_ops.cc index d8064819d361..d918c2af5308 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::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 36b5d2c24f64..a5887f37ce1e 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::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 a19bac9c5f6b..b0b176fc6564 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 99fb14a71d72..9d51a66c71f9 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(); }