projects / gcc.git / commitdiff
? search:
summary | shortlog | log | commit | commitdiff | tree
raw | patch | inline | side by side (parent: 958d63e)
PR libstdc++/83626 handle ENOENT due to filesystem race
authorJonathan Wakely <jwakely@redhat.com>
Fri, 5 Jan 2018 18:02:18 +0000 (18:02 +0000)
committerJonathan Wakely <redi@gcc.gnu.org>
Fri, 5 Jan 2018 18:02:18 +0000 (18:02 +0000)
PR libstdc++/83626
* src/filesystem/ops.cc (remove(const path&, error_code&)): Do not
report an error for ENOENT.
(remove_all(const path&)): Fix type of result variable.
(remove_all(const path&, error_code&)): Use non-throwing increment
for directory iterator. Call POSIX remove directly to avoid redundant
calls to symlink_status. Do not report errors for ENOENT.
* src/filesystem/std-ops.cc: Likewise.
* testsuite/27_io/filesystem/operations/remove_all.cc: Test throwing
overload.
* testsuite/experimental/filesystem/operations/remove_all.cc:
Likewise.

From-SVN: r256283

libstdc++-v3/ChangeLog patch | blob | history
libstdc++-v3/src/filesystem/ops.cc patch | blob | history
libstdc++-v3/src/filesystem/std-ops.cc patch | blob | history
libstdc++-v3/testsuite/27_io/filesystem/operations/remove_all.cc patch | blob | history
libstdc++-v3/testsuite/experimental/filesystem/operations/remove_all.cc patch | blob | history

diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog
index 82de1ff85d009bd433f05cca982223a5788a2dbd..fb2eb63a6936b9e4147d9ead1f3d71d7753265db 100644 (file)
--- a/libstdc++-v3/ChangeLog
+++ b/libstdc++-v3/ChangeLog
@@ -1,3 +1,18 @@
+2018-01-05  Jonathan Wakely  <jwakely@redhat.com>
+
+       PR libstdc++/83626
+       * src/filesystem/ops.cc (remove(const path&, error_code&)): Do not
+       report an error for ENOENT.
+       (remove_all(const path&)): Fix type of result variable.
+       (remove_all(const path&, error_code&)): Use non-throwing increment
+       for directory iterator. Call POSIX remove directly to avoid redundant
+       calls to symlink_status. Do not report errors for ENOENT.
+       * src/filesystem/std-ops.cc: Likewise.
+       * testsuite/27_io/filesystem/operations/remove_all.cc: Test throwing
+       overload.
+       * testsuite/experimental/filesystem/operations/remove_all.cc:
+       Likewise.
+
 2018-01-04  Jonathan Wakely  <jwakely@redhat.com>
 
        PR libstdc++/83626
diff --git a/libstdc++-v3/src/filesystem/ops.cc b/libstdc++-v3/src/filesystem/ops.cc
index 5a7cb599bb678f52a65f869cb448c987a6761f22..3690fb94d63d7031dfef08eca17d0ca65a521122 100644 (file)
--- a/libstdc++-v3/src/filesystem/ops.cc
+++ b/libstdc++-v3/src/filesystem/ops.cc
@@ -1025,13 +1025,16 @@ fs::remove(const path& p, error_code& ec) noexcept
       ec.clear();
       return false; // Nothing to do, not an error.
     }
-  if (::remove(p.c_str()) != 0)
+  if (::remove(p.c_str()) == 0)
     {
-      ec.assign(errno, std::generic_category());
-      return false;
+      ec.clear();
+      return true;
     }
-  ec.clear();
-  return true;
+  else if (errno == ENOENT)
+    ec.clear();
+  else
+    ec.assign(errno, std::generic_category());
+  return false;
 }
 
 
@@ -1039,7 +1042,7 @@ std::uintmax_t
 fs::remove_all(const path& p)
 {
   error_code ec;
-  bool result = remove_all(p, ec);
+  const auto result = remove_all(p, ec);
   if (ec)
     _GLIBCXX_THROW_OR_ABORT(filesystem_error("cannot remove all", p, ec));
   return result;
@@ -1051,15 +1054,29 @@ fs::remove_all(const path& p, error_code& ec) noexcept
   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)
-      count += fs::remove_all(d->path(), ec);
-  if (!ec && fs::remove(p, ec))
+    {
+      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 (::remove(p.c_str()) == 0)
     ++count;
-  if (ec)
-    return -1;
+  else if (errno != ENOENT)
+    {
+      ec.assign(errno, std::generic_category());
+      return -1;
+    }
   return count;
 }
 
diff --git a/libstdc++-v3/src/filesystem/std-ops.cc b/libstdc++-v3/src/filesystem/std-ops.cc
index 6ff280a9c7614ab340a1e9be1b33e7f13bf57c65..2411bbb797713a5c5687c5a7cac82574224bf2ad 100644 (file)
--- a/libstdc++-v3/src/filesystem/std-ops.cc
+++ b/libstdc++-v3/src/filesystem/std-ops.cc
@@ -1274,13 +1274,16 @@ fs::remove(const path& p, error_code& ec) noexcept
       ec.clear();
       return false; // Nothing to do, not an error.
     }
-  if (::remove(p.c_str()) != 0)
+  if (::remove(p.c_str()) == 0)
     {
-      ec.assign(errno, std::generic_category());
-      return false;
+      ec.clear();
+      return true;
     }
-  ec.clear();
-  return true;
+  else if (errno == ENOENT)
+    ec.clear();
+  else
+    ec.assign(errno, std::generic_category());
+  return false;
 }
 
 
@@ -1288,7 +1291,7 @@ std::uintmax_t
 fs::remove_all(const path& p)
 {
   error_code ec;
-  bool result = remove_all(p, ec);
+  const auto result = remove_all(p, ec);
   if (ec)
     _GLIBCXX_THROW_OR_ABORT(filesystem_error("cannot remove all", p, ec));
   return result;
@@ -1300,15 +1303,29 @@ 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)
-      count += fs::remove_all(d->path(), ec);
-  if (!ec && fs::remove(p, ec))
+    {
+      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 (::remove(p.c_str()) == 0)
     ++count;
-  if (ec)
-    return -1;
+  else if (errno != ENOENT)
+    {
+      ec.assign(errno, std::generic_category());
+      return -1;
+    }
   return count;
 }
 
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 8ee5c4d39c1577b9b197c6b60869cfbd125a25b9..633cde57243f55854907d1956abf2fb6aa56e89c 100644 (file)
--- a/libstdc++-v3/testsuite/27_io/filesystem/operations/remove_all.cc
+++ b/libstdc++-v3/testsuite/27_io/filesystem/operations/remove_all.cc
@@ -85,8 +85,28 @@ test01()
   b2.path.clear();
 }
 
+void
+test02()
+{
+  const auto dir = __gnu_test::nonexistent_path();
+  create_directories(dir/"a/b/c");
+  std::uintmax_t n = remove_all(dir/"a");
+  VERIFY( n == 3 );
+  VERIFY( exists(dir) );
+  VERIFY( !exists(dir/"a") );
+
+  n = remove_all(dir/"a");
+  VERIFY( n == 0 );
+  VERIFY( exists(dir) );
+
+  n = remove_all(dir);
+  VERIFY( n == 1 );
+  VERIFY( !exists(dir) );
+}
+
 int
 main()
 {
   test01();
+  test02();
 }
diff --git a/libstdc++-v3/testsuite/experimental/filesystem/operations/remove_all.cc b/libstdc++-v3/testsuite/experimental/filesystem/operations/remove_all.cc
index fa146b4317cc49810dd8acc2644bee66e8e198bc..67f6e989d27a4ed6e67de76f179d40dd4fd02171 100644 (file)
--- a/libstdc++-v3/testsuite/experimental/filesystem/operations/remove_all.cc
+++ b/libstdc++-v3/testsuite/experimental/filesystem/operations/remove_all.cc
@@ -85,8 +85,28 @@ test01()
   b2.path.clear();
 }
 
+void
+test02()
+{
+  const auto dir = __gnu_test::nonexistent_path();
+  create_directories(dir/"a/b/c");
+  std::uintmax_t n = remove_all(dir/"a");
+  VERIFY( n == 3 );
+  VERIFY( exists(dir) );
+  VERIFY( !exists(dir/"a") );
+
+  n = remove_all(dir/"a");
+  VERIFY( n == 0 );
+  VERIFY( exists(dir) );
+
+  n = remove_all(dir);
+  VERIFY( n == 1 );
+  VERIFY( !exists(dir) );
+}
+
 int
 main()
 {
   test01();
+  test02();
 }