mirror/gcc
2
0
Fork 0
mirror of git://gcc.gnu.org/git/gcc.git synced 2025-04-04 15:41:21 +08:00
Code Issues Packages Projects Releases Wiki Activity

libstdc++: Fix error handling in filesystem::remove_all (PR93201)

Browse Source 
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).

It also improves the error reporting so that the name of the file that
could not be removed is included in the filesystem_error exception. This
is done by introducing a new helper type for reporting errors with
additional context and a new function that uses that type. Then the
overload of std::filesystem::remove_all that throws an exception can use
the new function to ensure any exception contains the additional
information.

For std::experimental::filesystem::remove_all just fix the bug where
errors are ignored.

	PR libstdc++/93201
	* src/c++17/fs_ops.cc (do_remove_all): New function implementing more
	detailed error reporting for remove_all. Check result of recursive
	call before incrementing iterator.
	(remove_all(const path&), remove_all(const path&, error_code&)): Use
	do_remove_all.
	* src/filesystem/ops.cc (remove_all(const path&, error_code&)): Check
	result of recursive call before incrementing iterator.
	* testsuite/27_io/filesystem/operations/remove_all.cc: Check errors
	are reported correctly.
	* testsuite/experimental/filesystem/operations/remove_all.cc: Likewise.

From-SVN: r280014
This commit is contained in:
Jonathan Wakely 2020-01-08 16:44:45 +00:00 committed by Jonathan Wakely
parent face749a49
commit fff148b787
5 changed files with 182 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
libstdc++-v3/ChangeLog
View File

@ -1,3 +1,17 @@
2020-01-08 Jonathan Wakely <jwakely@redhat.com>
PR libstdc++/93201
* src/c++17/fs_ops.cc (do_remove_all): New function implementing more
detailed error reporting for remove_all. Check result of recursive
call before incrementing iterator.
(remove_all(const path&), remove_all(const path&, error_code&)): Use
do_remove_all.
* src/filesystem/ops.cc (remove_all(const path&, error_code&)): Check
result of recursive call before incrementing iterator.
* testsuite/27_io/filesystem/operations/remove_all.cc: Check errors
are reported correctly.
* testsuite/experimental/filesystem/operations/remove_all.cc: Likewise.
2020-01-07 Thomas Rodgers <trodgers@redhat.com>
* include/std/condition_variable

115
libstdc++-v3/src/c++17/fs_ops.cc
View File

@ -1275,42 +1275,105 @@ fs::remove(const path& p, error_code& ec) noexcept
return false;
}
namespace std::filesystem
{
namespace
{
struct ErrorReporter
{
explicit
ErrorReporter(error_code& ec) : code(&ec)
{ }
explicit
ErrorReporter(const char* s, const path& p)
: code(nullptr), msg(s), path1(&p)
{ }
error_code* code;
const char* msg;
const path* path1;
void
report(const error_code& ec) const
{
if (code)
*code = ec;
else
_GLIBCXX_THROW_OR_ABORT(filesystem_error(msg, *path1, ec));
}
void
report(const error_code& ec, const path& path2) const
{
if (code)
*code = ec;
else if (path2 != *path1)
_GLIBCXX_THROW_OR_ABORT(filesystem_error(msg, *path1, path2, ec));
else
_GLIBCXX_THROW_OR_ABORT(filesystem_error(msg, *path1, ec));
}
};
uintmax_t
do_remove_all(const path& p, const ErrorReporter& err)
{
error_code ec;
const auto s = symlink_status(p, ec);
if (!status_known(s))
{
if (ec)
err.report(ec, p);
return -1;
}
ec.clear();
if (s.type() == file_type::not_found)
return 0;
uintmax_t count = 0;
if (s.type() == file_type::directory)
{
directory_iterator d(p, ec), end;
while (d != end)
{
const auto removed = fs::do_remove_all(d->path(), err);
if (removed == numeric_limits<uintmax_t>::max())
return -1;
count += removed;
d.increment(ec);
if (ec)
{
err.report(ec, p);
return -1;
}
}
}
if (fs::remove(p, ec))
++count;
if (ec)
{
err.report(ec, p);
return -1;
}
return count;
}
}
}
std::uintmax_t
fs::remove_all(const path& p)
{
error_code ec;
const auto result = remove_all(p, ec);
if (ec)
_GLIBCXX_THROW_OR_ABORT(filesystem_error("cannot remove all", p, ec));
return result;
return fs::do_remove_all(p, ErrorReporter{"cannot remove all", p});
}
std::uintmax_t
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.increment(ec))
count += fs::remove_all(d->path(), ec);
if (ec.value() == ENOENT)
ec.clear();
else if (ec)
return -1;
}
if (fs::remove(p, ec))
++count;
return ec ? -1 : count;
return fs::do_remove_all(p, ErrorReporter{ec});
}
void

17
libstdc++-v3/src/filesystem/ops.cc
View File

@ -1099,12 +1099,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))

35
libstdc++-v3/testsuite/27_io/filesystem/operations/remove_all.cc
View File

@ -140,10 +140,45 @@ 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 );
// Second path is the first file that couldn't be removed
VERIFY( e.path2() == dir/"file" );
}
fs::permissions(dir, fs::perms::owner_write, fs::perm_options::add);
#endif
}
int
main()
{
test01();
test02();
test03();
test04();
}

33
libstdc++-v3/testsuite/experimental/filesystem/operations/remove_all.cc
View File

@ -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();
}