[libcxx-commits] [libcxx] [libc++] Fix filesystem::remove_all() on FreeBSD (PR #79540)

[Mark Johnston via libcxx-commits]

https://github.com/markjdb updated https://github.com/llvm/llvm-project/pull/79540

>From 5f187e5a28372ae323efffba485ecdc4f6b624d1 Mon Sep 17 00:00:00 2001
From: Mark Johnston <markj at FreeBSD.org>
Date: Thu, 25 Jan 2024 21:29:01 -0500
Subject: [PATCH 1/2] [libc++] Fix filesystem::remove_all() on FreeBSD

remove_all_impl() opens the target path with O_NOFOLLOW, which fails if
the target is a symbolic link.  On FreeBSD, rather than returning ELOOP,
openat() returns EMLINK.  This is unlikely to change for compatibility
reasons, see https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=214633 .

Thus, check for EMLINK as well.
---
 libcxx/src/filesystem/operations.cpp | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/libcxx/src/filesystem/operations.cpp b/libcxx/src/filesystem/operations.cpp
index 6253d1551b062e..62bb248d5eca9b 100644
--- a/libcxx/src/filesystem/operations.cpp
+++ b/libcxx/src/filesystem/operations.cpp
@@ -815,8 +815,9 @@ uintmax_t remove_all_impl(int parent_directory, const path& p, error_code& ec) {
 
   // If opening `p` failed because it wasn't a directory, remove it as
   // a normal file instead. Note that `openat()` can return either ENOTDIR
-  // or ELOOP depending on the exact reason of the failure.
-  if (ec == errc::not_a_directory || ec == errc::too_many_symbolic_link_levels) {
+  // or ELOOP depending on the exact reason of the failure. On FreeBSD it
+  // may return EMLINK instead of ELOOP, contradicting POSIX.
+  if (ec == errc::not_a_directory || ec == errc::too_many_symbolic_link_levels || ec == errc::too_many_links) {
     ec.clear();
     if (::unlinkat(parent_directory, p.c_str(), /* flags = */ 0) == -1) {
       ec = detail::capture_errno();

>From 4c53574cf582c3d5f14d82ecb3c14199c474b26a Mon Sep 17 00:00:00 2001
From: Mark Johnston <markj at FreeBSD.org>
Date: Fri, 26 Jan 2024 11:48:04 -0500
Subject: [PATCH 2/2] [libc++][test] Remove XFAIL annotation for the
 remove_all() test

It passes on FreeBSD now that remove_all_impl() handles FreeBSD's
non-standard error value when opening a symlink with O_NOFOLLOW.
---
 .../fs.op.funcs/fs.op.remove_all/remove_all.pass.cpp            | 2 --
 1 file changed, 2 deletions(-)

diff --git a/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.remove_all/remove_all.pass.cpp b/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.remove_all/remove_all.pass.cpp
index eaa8044397ed85..5cdbd087843165 100644
--- a/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.remove_all/remove_all.pass.cpp
+++ b/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.remove_all/remove_all.pass.cpp
@@ -6,8 +6,6 @@
 //
 //===----------------------------------------------------------------------===//
 
-// XFAIL: LIBCXX-FREEBSD-FIXME
-
 // UNSUPPORTED: c++03, c++11, c++14
 // UNSUPPORTED: no-filesystem
 // UNSUPPORTED: availability-filesystem-missing