From: Jonathan Wakely Date: Thu, 4 Jan 2018 22:58:59 +0000 (+0000) Subject: PR libstdc++/83626 Don't throw for remove("") and remove_all("") X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=994844d36ad5215af397fa07fbb8c4f85f9e743f;p=gcc.git PR libstdc++/83626 Don't throw for remove("") and remove_all("") PR libstdc++/83626 * src/filesystem/ops.cc (remove(const path&, error_code&))): Remove redundant call to ec.clear(). (remove_all(const path&, error_code&))): Do not return an error for non-existent paths. * src/filesystem/std-ops.cc: Likewise. * testsuite/27_io/filesystem/operations/remove.cc: New test. * testsuite/27_io/filesystem/operations/remove_all.cc: Fix expected results for non-existent paths. * testsuite/experimental/filesystem/operations/remove.cc: New test. * testsuite/experimental/filesystem/operations/remove_all.cc: Fix expected results for non-existent paths. From-SVN: r256269 --- diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog index 83361d45188..82de1ff85d0 100644 --- a/libstdc++-v3/ChangeLog +++ b/libstdc++-v3/ChangeLog @@ -1,5 +1,18 @@ 2018-01-04 Jonathan Wakely + PR libstdc++/83626 + * src/filesystem/ops.cc (remove(const path&, error_code&))): Remove + redundant call to ec.clear(). + (remove_all(const path&, error_code&))): Do not return an error for + non-existent paths. + * src/filesystem/std-ops.cc: Likewise. + * testsuite/27_io/filesystem/operations/remove.cc: New test. + * testsuite/27_io/filesystem/operations/remove_all.cc: Fix expected + results for non-existent paths. + * testsuite/experimental/filesystem/operations/remove.cc: New test. + * testsuite/experimental/filesystem/operations/remove_all.cc: Fix + expected results for non-existent paths. + * include/bits/fs_ops.h (exists(const path&, error_code&))): Only check status_known once. * include/experimental/bits/fs_ops.h: Likewise. diff --git a/libstdc++-v3/src/filesystem/ops.cc b/libstdc++-v3/src/filesystem/ops.cc index 56b1432036f..5a7cb599bb6 100644 --- a/libstdc++-v3/src/filesystem/ops.cc +++ b/libstdc++-v3/src/filesystem/ops.cc @@ -1009,7 +1009,7 @@ fs::remove(const path& p) { error_code ec; bool result = fs::remove(p, ec); - if (ec.value()) + if (ec) _GLIBCXX_THROW_OR_ABORT(filesystem_error("cannot remove", p, ec)); return result; } @@ -1017,17 +1017,21 @@ fs::remove(const path& p) bool fs::remove(const path& p, error_code& ec) noexcept { - if (exists(symlink_status(p, ec))) + const auto s = symlink_status(p, ec); + if (!status_known(s)) + return false; + if (s.type() == file_type::not_found) { - if (::remove(p.c_str()) == 0) - { - ec.clear(); - return true; - } - else - ec.assign(errno, std::generic_category()); + ec.clear(); + return false; // Nothing to do, not an error. } - return false; + if (::remove(p.c_str()) != 0) + { + ec.assign(errno, std::generic_category()); + return false; + } + ec.clear(); + return true; } @@ -1036,7 +1040,7 @@ fs::remove_all(const path& p) { error_code ec; bool result = remove_all(p, ec); - if (ec.value()) + if (ec) _GLIBCXX_THROW_OR_ABORT(filesystem_error("cannot remove all", p, ec)); return result; } @@ -1044,14 +1048,19 @@ fs::remove_all(const path& p) std::uintmax_t fs::remove_all(const path& p, error_code& ec) noexcept { - auto fs = symlink_status(p, ec); + const auto s = symlink_status(p, ec); + if (!status_known(s)) + return -1; + ec.clear(); uintmax_t count = 0; - if (ec.value() == 0 && fs.type() == file_type::directory) - for (directory_iterator d(p, ec), end; ec.value() == 0 && d != end; ++d) + 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.value()) + if (!ec && fs::remove(p, ec)) + ++count; + if (ec) return -1; - return fs::remove(p, ec) ? ++count : -1; // fs:remove() calls ec.clear() + return count; } void diff --git a/libstdc++-v3/src/filesystem/std-ops.cc b/libstdc++-v3/src/filesystem/std-ops.cc index 8392f6fc835..6ff280a9c76 100644 --- a/libstdc++-v3/src/filesystem/std-ops.cc +++ b/libstdc++-v3/src/filesystem/std-ops.cc @@ -1266,17 +1266,21 @@ fs::remove(const path& p) bool fs::remove(const path& p, error_code& ec) noexcept { - if (exists(symlink_status(p, ec))) + const auto s = symlink_status(p, ec); + if (!status_known(s)) + return false; + if (s.type() == file_type::not_found) { - if (::remove(p.c_str()) == 0) - { - ec.clear(); - return true; - } - else - ec.assign(errno, std::generic_category()); + ec.clear(); + return false; // Nothing to do, not an error. } - return false; + if (::remove(p.c_str()) != 0) + { + ec.assign(errno, std::generic_category()); + return false; + } + ec.clear(); + return true; } @@ -1293,14 +1297,19 @@ fs::remove_all(const path& p) std::uintmax_t fs::remove_all(const path& p, error_code& ec) { - auto fs = symlink_status(p, ec); + const auto s = symlink_status(p, ec); + if (!status_known(s)) + return -1; + ec.clear(); uintmax_t count = 0; - if (!ec && fs.type() == file_type::directory) + 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)) + ++count; if (ec) return -1; - return fs::remove(p, ec) ? ++count : -1; // fs:remove() calls ec.clear() + return count; } void diff --git a/libstdc++-v3/testsuite/27_io/filesystem/operations/remove.cc b/libstdc++-v3/testsuite/27_io/filesystem/operations/remove.cc new file mode 100644 index 00000000000..ef9f06d7300 --- /dev/null +++ b/libstdc++-v3/testsuite/27_io/filesystem/operations/remove.cc @@ -0,0 +1,100 @@ +// Copyright (C) 2018 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. + +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// . + +// { dg-options "-std=gnu++17 -lstdc++fs" } +// { dg-do run { target c++17 } } +// { dg-require-filesystem-ts "" } + +#include +#include +#include + +namespace fs = std::filesystem; + +void +test01() +{ + std::error_code ec; + const std::error_code bad_ec = make_error_code(std::errc::invalid_argument); + bool n; + + n = fs::remove("", ec); + VERIFY( !ec ); // This seems odd, but is what the standard requires. + VERIFY( !n ); + + auto p = __gnu_test::nonexistent_path(); + ec = bad_ec; + n = remove(p, ec); + VERIFY( !ec ); + VERIFY( !n ); + + auto link = __gnu_test::nonexistent_path(); + create_symlink(p, link); // dangling symlink + ec = bad_ec; + n = remove(link, ec); + VERIFY( !ec ); + VERIFY( n ); + VERIFY( !exists(symlink_status(link)) ); + + __gnu_test::scoped_file f(p); + create_symlink(p, link); + ec = bad_ec; + n = remove(link, ec); + VERIFY( !ec ); + VERIFY( n ); + VERIFY( !exists(symlink_status(link)) ); // The symlink is removed, but + VERIFY( exists(p) ); // its target is not. + + ec = bad_ec; + n = remove(p, ec); + VERIFY( !ec ); + VERIFY( n ); + VERIFY( !exists(symlink_status(p)) ); + + const auto dir = __gnu_test::nonexistent_path(); + create_directories(dir/"a/b"); + ec.clear(); + n = remove(dir/"a", ec); + VERIFY( ec ); + VERIFY( !n ); + VERIFY( exists(dir/"a/b") ); + + permissions(dir, fs::perms::none, ec); + if (!ec) + { + ec.clear(); + n = remove(dir/"a/b", ec); + VERIFY( ec ); + VERIFY( !n ); + permissions(dir, fs::perms::owner_all, ec); + } + + ec = bad_ec; + n = remove(dir/"a/b", ec); + VERIFY( !ec ); + VERIFY( n ); + VERIFY( !exists(dir/"a/b") ); + + remove(dir/"a", ec); + remove(dir, ec); +} + +int +main() +{ + test01(); +} 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 9cdc86fe9c3..8ee5c4d39c1 100644 --- a/libstdc++-v3/testsuite/27_io/filesystem/operations/remove_all.cc +++ b/libstdc++-v3/testsuite/27_io/filesystem/operations/remove_all.cc @@ -29,19 +29,19 @@ void test01() { std::error_code ec; + const std::error_code bad_ec = make_error_code(std::errc::invalid_argument); std::uintmax_t n; n = fs::remove_all("", ec); - VERIFY( ec ); - VERIFY( n == std::uintmax_t(-1) ); + VERIFY( !ec ); // This seems odd, but is what the standard requires. + VERIFY( n == 0 ); auto p = __gnu_test::nonexistent_path(); - ec.clear(); + ec = bad_ec; n = remove_all(p, ec); - VERIFY( ec ); - VERIFY( n == std::uintmax_t(-1) ); + VERIFY( !ec ); + VERIFY( n == 0 ); - const auto bad_ec = ec; auto link = __gnu_test::nonexistent_path(); create_symlink(p, link); // dangling symlink ec = bad_ec; @@ -59,7 +59,7 @@ test01() VERIFY( !exists(symlink_status(link)) ); // The symlink is removed, but VERIFY( exists(p) ); // its target is not. - auto dir = __gnu_test::nonexistent_path(); + const auto dir = __gnu_test::nonexistent_path(); create_directories(dir/"a/b/c"); ec = bad_ec; n = remove_all(dir/"a", ec); diff --git a/libstdc++-v3/testsuite/experimental/filesystem/operations/remove.cc b/libstdc++-v3/testsuite/experimental/filesystem/operations/remove.cc new file mode 100644 index 00000000000..8d1801099a2 --- /dev/null +++ b/libstdc++-v3/testsuite/experimental/filesystem/operations/remove.cc @@ -0,0 +1,100 @@ +// Copyright (C) 2018 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. + +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// . + +// { dg-options "-lstdc++fs" } +// { dg-do run { target c++11 } } +// { dg-require-filesystem-ts "" } + +#include +#include +#include + +namespace fs = std::experimental::filesystem; + +void +test01() +{ + std::error_code ec; + const std::error_code bad_ec = make_error_code(std::errc::invalid_argument); + bool n; + + n = fs::remove("", ec); + VERIFY( !ec ); // This seems odd, but is what the standard requires. + VERIFY( !n ); + + auto p = __gnu_test::nonexistent_path(); + ec = bad_ec; + n = remove(p, ec); + VERIFY( !ec ); + VERIFY( !n ); + + auto link = __gnu_test::nonexistent_path(); + create_symlink(p, link); // dangling symlink + ec = bad_ec; + n = remove(link, ec); + VERIFY( !ec ); + VERIFY( n ); + VERIFY( !exists(symlink_status(link)) ); + + __gnu_test::scoped_file f(p); + create_symlink(p, link); + ec = bad_ec; + n = remove(link, ec); + VERIFY( !ec ); + VERIFY( n ); + VERIFY( !exists(symlink_status(link)) ); // The symlink is removed, but + VERIFY( exists(p) ); // its target is not. + + ec = bad_ec; + n = remove(p, ec); + VERIFY( !ec ); + VERIFY( n ); + VERIFY( !exists(symlink_status(p)) ); + + const auto dir = __gnu_test::nonexistent_path(); + create_directories(dir/"a/b"); + ec.clear(); + n = remove(dir/"a", ec); + VERIFY( ec ); + VERIFY( !n ); + VERIFY( exists(dir/"a/b") ); + + permissions(dir, fs::perms::none, ec); + if (!ec) + { + ec.clear(); + n = remove(dir/"a/b", ec); + VERIFY( ec ); + VERIFY( !n ); + permissions(dir, fs::perms::owner_all, ec); + } + + ec = bad_ec; + n = remove(dir/"a/b", ec); + VERIFY( !ec ); + VERIFY( n ); + VERIFY( !exists(dir/"a/b") ); + + remove(dir/"a", ec); + remove(dir, ec); +} + +int +main() +{ + test01(); +} diff --git a/libstdc++-v3/testsuite/experimental/filesystem/operations/remove_all.cc b/libstdc++-v3/testsuite/experimental/filesystem/operations/remove_all.cc index 983998d78fa..fa146b4317c 100644 --- a/libstdc++-v3/testsuite/experimental/filesystem/operations/remove_all.cc +++ b/libstdc++-v3/testsuite/experimental/filesystem/operations/remove_all.cc @@ -29,19 +29,19 @@ void test01() { std::error_code ec; + const std::error_code bad_ec = make_error_code(std::errc::invalid_argument); std::uintmax_t n; n = fs::remove_all("", ec); - VERIFY( ec ); - VERIFY( n == std::uintmax_t(-1) ); + VERIFY( !ec ); // This seems odd, but is what the TS requires. + VERIFY( n == 0 ); auto p = __gnu_test::nonexistent_path(); - ec.clear(); + ec = bad_ec; n = remove_all(p, ec); - VERIFY( ec ); - VERIFY( n == std::uintmax_t(-1) ); + VERIFY( !ec ); + VERIFY( n == 0 ); - const auto bad_ec = ec; auto link = __gnu_test::nonexistent_path(); create_symlink(p, link); // dangling symlink ec = bad_ec; @@ -59,7 +59,7 @@ test01() VERIFY( !exists(symlink_status(link)) ); // The symlink is removed, but VERIFY( exists(p) ); // its target is not. - auto dir = __gnu_test::nonexistent_path(); + const auto dir = __gnu_test::nonexistent_path(); create_directories(dir/"a/b/c"); ec = bad_ec; n = remove_all(dir/"a", ec);