Fix for remove_all handling when calling for non-existent or regular …

…file. (#6)
gulrak · Mar 5, 2019 · 1349d3d · 1349d3d
1 parent 71d05aa
commit 1349d3d
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 16 deletions.
diff --git a/README.md b/README.md
@@ -345,6 +345,16 @@ to the expected behavior.
 
 ## Release Notes
 
+### v1.0.8 (wip)
+
+* Bugfix for ([#6](https://github.com/gulrak/filesystem/issues/6)), where
+  `ghc::filesystem::remove()` and `ghc::filesystem::remove_all()` both are
+  now able to remove a single file and both will not raise an error if the
+  path doesn't exist.
+* Merged pull request ([#7](https://github.com/gulrak/filesystem/pull/7)),
+  a typo leading to setting error code instead of comparing it in
+  `ghc::filesystem::remove()` under Windows.
+
 ### [v1.0.6](https://github.com/gulrak/filesystem/releases/tag/v1.0.6)
 
 * Bugfix for ([#4](https://github.com/gulrak/filesystem/issues/4)), missing error_code

diff --git a/filesystem.h b/filesystem.h
@@ -3660,7 +3660,6 @@ inline bool remove(const path& p, std::error_code& ec) noexcept
         }
     }
 #else
-
     if (::remove(p.c_str()) == -1) {
         auto error = errno;
         if (error == ENOENT) {
@@ -3690,22 +3689,26 @@ inline uintmax_t remove_all(const path& p, std::error_code& ec) noexcept
         ec = detail::make_error_code(detail::portable_error::not_supported);
         return static_cast<uintmax_t>(-1);
     }
-    for (auto iter = directory_iterator(p, ec); iter != directory_iterator(); iter.increment(ec)) {
-        if(ec) {
-            break;
-        }
-        if (!iter->is_symlink() && iter->is_directory()) {
-            count += remove_all(iter->path(), ec);
-            if (ec) {
-                return static_cast<uintmax_t>(-1);
+    std::error_code tec;
+    auto fs = status(p, tec);
+    if(exists(fs) && is_directory(fs)) {
+        for (auto iter = directory_iterator(p, ec); iter != directory_iterator(); iter.increment(ec)) {
+            if(ec) {
+                break;
             }
-        }
-        else {
-            remove(iter->path(), ec);
-            if (ec) {
-                return static_cast<uintmax_t>(-1);
+            if (!iter->is_symlink() && iter->is_directory()) {
+                count += remove_all(iter->path(), ec);
+                if (ec) {
+                    return static_cast<uintmax_t>(-1);
+                }
+            }
+            else {
+                remove(iter->path(), ec);
+                if (ec) {
+                    return static_cast<uintmax_t>(-1);
+                }
+                ++count;
             }
-            ++count;
         }
     }
     if(!ec) {

diff --git a/test/filesystem_test.cpp b/test/filesystem_test.cpp
@@ -1984,6 +1984,7 @@ TEST_CASE("30.10.15.30 remove", "[filesystem][operations][fs.op.remove]")
     generateFile("foo");
     CHECK(fs::remove("foo"));
     CHECK(!fs::exists("foo"));
+    CHECK(!fs::remove("foo"));
     generateFile("foo");
     CHECK(fs::remove("foo", ec));
     CHECK(!fs::exists("foo"));
@@ -2004,13 +2005,17 @@ TEST_CASE("30.10.15.31 remove_all", "[filesystem][operations][fs.op.remove_all]"
 {
     TemporaryDirectory t(TempOpt::change_path);
     std::error_code ec;
+    generateFile("foo");
+    CHECK(fs::remove_all("foo", ec) == 1);
+    CHECK(!ec);
+    ec.clear();
     CHECK(fs::directory_iterator(t.path()) == fs::directory_iterator());
     fs::create_directories("dir1/dir1a");
     fs::create_directories("dir1/dir1b");
     generateFile("dir1/dir1a/f1");
     generateFile("dir1/dir1b/f2");
     CHECK_NOTHROW(fs::remove_all("dir1/non-existing", ec));
-    CHECK(ec);
+    CHECK(!ec);
     CHECK(fs::remove_all("dir1") == 5);
     CHECK(fs::directory_iterator(t.path()) == fs::directory_iterator());
 }