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

[Jonathan Wakely]

On 08/01/20 16:44 +0000, Jonathan Wakely wrote:
>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.

This is what I plan to commit to the branches. It doesn't improve the
error reporting, just fixes the bug. But that *does* still change the
observable behaviour when an error occurs (because now we return
immediately on the first error, instead of continuing to remove as
much as possible).

I've sent an email to the Library Evolution list to discuss the
desired behaviour. Does anybody here have an opinion? Should we
preserve the existing "keep going then report errors at the end"
behaviour for the branches, or just make this change?



-------------- next part --------------
A non-text attachment was scrubbed...
Name: patch.txt
Type: text/x-patch
Size: 5774 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20200108/52e0c69e/attachment.bin>