[libcxx-commits] [libcxx] [libc++] Make std::filesystem::canonical throw when given empty path (PR #77223)

via libcxx-commits libcxx-commits at lists.llvm.org
Sat Jan 6 22:33:57 PST 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-libcxx

Author: Leo (Yuki-cpp)

<details>
<summary>Changes</summary>

Fixes #<!-- -->77170 

As noted in the issue, `std::filesystem::canonical` returns the current path when given `""` while an error is expected.
To fix that, this PR adds a check for empty paths in `std::filesystem::canonical` and raise an error accordingly. Since `std::filesystem::weakly_canonical` uses this function and should not raise an error when given an empty path, it was updated to return an empty path instead of the results of canonical.

Note: Running clang-format on the [canonical.pass.cpp](https://github.com/llvm/llvm-project/compare/main...Yuki-cpp:llvm-project:fix-77170-canonical-should-throw?expand=1#diff-0111faeef5a805a4fa6b7fa5a93698f1009684f17dbf80698c81c70da7fcc284) file produced formatting different than the rest of the file. As such, to conform with the [Coding Standard](https://llvm.org/docs/CodingStandards.html) I haven't formatted that file automatically but rather kept the style of the file intact. Let me know if I missed something there and I'll gladly fix it.

Note 2: Might as well mention now that I do not have commit access

---
Full diff: https://github.com/llvm/llvm-project/pull/77223.diff


2 Files Affected:

- (modified) libcxx/src/filesystem/operations.cpp (+4-2) 
- (modified) libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.canonical/canonical.pass.cpp (+26) 


``````````diff
diff --git a/libcxx/src/filesystem/operations.cpp b/libcxx/src/filesystem/operations.cpp
index 6bee340e0d15c8..8f5b70a071373d 100644
--- a/libcxx/src/filesystem/operations.cpp
+++ b/libcxx/src/filesystem/operations.cpp
@@ -82,6 +82,8 @@ path __canonical(path const& orig_p, error_code* ec) {
   path cwd;
   ErrorHandler<path> err("canonical", ec, &orig_p, &cwd);
 
+  if (orig_p.empty())
+    return err.report(capture_errno());
   path p = __do_absolute(orig_p, &cwd, ec);
 #if (defined(_POSIX_VERSION) && _POSIX_VERSION >= 200112) || defined(_LIBCPP_WIN32API)
   std::unique_ptr<path::value_type, decltype(&::free)> hold(detail::realpath(p.c_str(), nullptr), &::free);
@@ -912,7 +914,7 @@ path __weakly_canonical(const path& p, error_code* ec) {
   ErrorHandler<path> err("weakly_canonical", ec, &p);
 
   if (p.empty())
-    return __canonical("", ec);
+    return __current_path(ec);
 
   path result;
   path tmp;
@@ -935,7 +937,7 @@ path __weakly_canonical(const path& p, error_code* ec) {
     --PP;
   }
   if (PP.State == PathParser::PS_BeforeBegin)
-    result = __canonical("", ec);
+    result = __current_path(ec);
   if (ec)
     ec->clear();
   if (DNEParts.empty())
diff --git a/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.canonical/canonical.pass.cpp b/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.canonical/canonical.pass.cpp
index 0098fe8ee698ef..000fb47096ba19 100644
--- a/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.canonical/canonical.pass.cpp
+++ b/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.canonical/canonical.pass.cpp
@@ -92,6 +92,31 @@ static void test_dne_path()
     }
 }
 
+static void test_empty_path()
+{
+    std::error_code ec = GetTestEC();
+    {
+        const path ret = canonical(path{}, ec);
+        assert(ec != GetTestEC());
+        assert(ec);
+        assert(ret == path{});
+    }
+    {
+        TEST_THROWS_TYPE(filesystem_error, canonical(path{}));
+    }
+
+    ec = GetTestEC();
+    {
+        const path ret = canonical("", ec);
+        assert(ec != GetTestEC());
+        assert(ec);
+        assert(ret == path{});
+    }
+    {
+        TEST_THROWS_TYPE(filesystem_error, canonical(""));
+    }
+}
+
 static void test_exception_contains_paths()
 {
 #ifndef TEST_HAS_NO_EXCEPTIONS
@@ -121,6 +146,7 @@ int main(int, char**) {
     signature_test();
     test_canonical();
     test_dne_path();
+    test_empty_path();
     test_exception_contains_paths();
 
     return 0;

``````````

</details>


https://github.com/llvm/llvm-project/pull/77223


More information about the libcxx-commits mailing list