[libc] Cleanup mman functions and tests

libc/src/sys/mman/linux/CMakeLists.txt

@@ -184,11 +184,10 @@ add_header_library(
   HDRS
     shm_common.h
   DEPENDS
+    libc.hdr.errno_macros
     libc.src.__support.CPP.array
     libc.src.__support.CPP.string_view
-    libc.src.__support.CPP.optional
-    libc.src.__support.common
-    libc.src.errno.errno
+    libc.src.__support.error_or
     libc.src.string.memory_utils.inline_memcpy
 )
 
@@ -199,8 +198,8 @@ add_entrypoint_object(
   HDRS
     ../shm_open.h
   DEPENDS
-    libc.src.fcntl.open
     libc.hdr.types.mode_t
+    libc.src.errno.errno
     .shm_common
 )
 
@@ -211,6 +210,6 @@ add_entrypoint_object(
   HDRS
     ../shm_unlink.h
   DEPENDS
-    libc.src.unistd.unlink
+    libc.src.errno.errno
     .shm_common
 )

libc/src/sys/mman/linux/shm_common.h

@@ -6,18 +6,13 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "hdr/errno_macros.h"
 #include "src/__support/CPP/array.h"
-#include "src/__support/CPP/optional.h"
 #include "src/__support/CPP/string_view.h"
-#include "src/__support/libc_errno.h"
+#include "src/__support/error_or.h"
 #include "src/__support/macros/config.h"
 #include "src/string/memory_utils/inline_memcpy.h"
 
-// TODO: clean this up.
-//  1. Change from optional to ErrorOr, and return the errno instead of setting
-//    it here.
-//  2. Replace inline memcpy with __builtin_memcpy
-
 // TODO: Get PATH_MAX via https://github.com/llvm/llvm-project/issues/85121
 #include <linux/limits.h>
 
@@ -28,24 +23,18 @@ namespace shm_common {
 LIBC_INLINE_VAR constexpr cpp::string_view SHM_PREFIX = "/dev/shm/";
 using SHMPath = cpp::array<char, NAME_MAX + SHM_PREFIX.size() + 1>;
 
-LIBC_INLINE cpp::optional<SHMPath> translate_name(cpp::string_view name) {
+LIBC_INLINE ErrorOr<SHMPath> translate_name(cpp::string_view name) {
   // trim leading slashes
   size_t offset = name.find_first_not_of('/');
-  if (offset == cpp::string_view::npos) {
-    libc_errno = EINVAL;
-    return cpp::nullopt;
-  }
+  if (offset == cpp::string_view::npos)
+    return Error(EINVAL);
   name = name.substr(offset);
 
   // check the name
-  if (name.size() > NAME_MAX) {
-    libc_errno = ENAMETOOLONG;
-    return cpp::nullopt;
-  }
-  if (name == "." || name == ".." || name.contains('/')) {
-    libc_errno = EINVAL;
-    return cpp::nullopt;
-  }
+  if (name.size() > NAME_MAX)
+    return Error(ENAMETOOLONG);
+  if (name == "." || name == ".." || name.contains('/'))
+    return Error(EINVAL);
 
   // prepend the prefix
   SHMPath buffer;

libc/src/sys/mman/linux/shm_open.cpp

@@ -7,9 +7,11 @@
 //===----------------------------------------------------------------------===//
 
 #include "src/sys/mman/shm_open.h"
+
 #include "hdr/fcntl_macros.h"
 #include "hdr/types/mode_t.h"
 #include "src/__support/OSUtil/fcntl.h"
+#include "src/__support/libc_errno.h"
 #include "src/__support/macros/config.h"
 #include "src/sys/mman/linux/shm_common.h"
 
@@ -18,17 +20,19 @@ namespace LIBC_NAMESPACE_DECL {
 static constexpr int DEFAULT_OFLAGS = O_NOFOLLOW | O_CLOEXEC | O_NONBLOCK;
 
 LLVM_LIBC_FUNCTION(int, shm_open, (const char *name, int oflags, mode_t mode)) {
-  if (cpp::optional<shm_common::SHMPath> buffer =
-          shm_common::translate_name(name)) {
-    auto result = internal::open(buffer->data(), oflags | DEFAULT_OFLAGS, mode);
+  auto path_result = shm_common::translate_name(name);
+  if (!path_result.has_value()) {
+    libc_errno = path_result.error();
+    return -1;
+  }
 
-    if (!result.has_value()) {
-      libc_errno = result.error();
-      return -1;
-    }
-    return result.value();
+  auto open_result =
+      internal::open(path_result->data(), oflags | DEFAULT_OFLAGS, mode);
+  if (!open_result.has_value()) {
+    libc_errno = open_result.error();
+    return -1;
   }
-  return -1;
+  return open_result.value();
 }
 
 } // namespace LIBC_NAMESPACE_DECL

libc/src/sys/mman/linux/shm_unlink.cpp

@@ -7,20 +7,38 @@
 //===----------------------------------------------------------------------===//
 
 #include "src/sys/mman/shm_unlink.h"
+
+#include "hdr/fcntl_macros.h"
+#include "src/__support/OSUtil/syscall.h" // For internal syscall function.
+#include "src/__support/libc_errno.h"     // For internal errno.
 #include "src/__support/macros/config.h"
 #include "src/sys/mman/linux/shm_common.h"
-#include "src/unistd/unlink.h"
+#include <sys/syscall.h> // For SYS_unlink, SYS_unlinkat
 
 namespace LIBC_NAMESPACE_DECL {
 
-// TODO: stop calling the public unlink function. It should be calling an
-// internal shared utility.
+// TODO: move the unlink syscall to a shared utility.
 
 LLVM_LIBC_FUNCTION(int, shm_unlink, (const char *name)) {
-  if (cpp::optional<shm_common::SHMPath> buffer =
-          shm_common::translate_name(name))
-    return LIBC_NAMESPACE::unlink(buffer->data());
-  return -1;
+  auto path_result = shm_common::translate_name(name);
+  if (!path_result.has_value()) {
+    libc_errno = path_result.error();
+    return -1;
+  }
+#ifdef SYS_unlink
+  int ret = LIBC_NAMESPACE::syscall_impl<int>(SYS_unlink, path_result->data());
+#elif defined(SYS_unlinkat)
+  int ret = LIBC_NAMESPACE::syscall_impl<int>(SYS_unlinkat, AT_FDCWD,
+                                              path_result->data(), 0);
+#else
+#error "unlink and unlinkat syscalls not available."
+#endif
+
+  if (ret < 0) {
+    libc_errno = -ret;
+    return -1;
+  }
+  return ret;
 }
 
 } // namespace LIBC_NAMESPACE_DECL

libc/test/src/sys/mman/linux/madvise_test.cpp

@@ -13,8 +13,6 @@
 #include "test/UnitTest/ErrnoSetterMatcher.h"
 #include "test/UnitTest/Test.h"
 
-#include <sys/mman.h>
-
 using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Fails;
 using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Succeeds;
 using LlvmLibcMadviseTest = LIBC_NAMESPACE::testing::ErrnoCheckingTest;

libc/test/src/sys/mman/linux/mincore_test.cpp

@@ -6,7 +6,6 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "src/__support/OSUtil/syscall.h" // For internal syscall function.
 #include "src/sys/mman/madvise.h"
 #include "src/sys/mman/mincore.h"
 #include "src/sys/mman/mlock.h"
@@ -18,10 +17,6 @@
 #include "test/UnitTest/ErrnoSetterMatcher.h"
 #include "test/UnitTest/Test.h"
 
-#include <sys/mman.h>
-#include <sys/syscall.h>
-#include <unistd.h>
-
 using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Fails;
 using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Succeeds;
 using LlvmLibcMincoreTest = LIBC_NAMESPACE::testing::ErrnoCheckingTest;

libc/test/src/sys/mman/linux/mlock_test.cpp

@@ -6,6 +6,10 @@
 //
 //===----------------------------------------------------------------------===//
 
+// TODO: Simplify these tests and split them up. mlock, mlock2, mlockall,
+// munlock, and munlockall should have separate test files which only need to
+// check our code paths (succeeds and errors).
+
 #include "src/__support/OSUtil/syscall.h" // For internal syscall function.
 #include "src/__support/libc_errno.h"
 #include "src/sys/mman/madvise.h"
@@ -24,10 +28,7 @@
 #include "test/UnitTest/Test.h"
 
 #include <linux/capability.h>
-#include <sys/mman.h>
-#include <sys/resource.h>
 #include <sys/syscall.h>
-#include <unistd.h>
 
 using namespace LIBC_NAMESPACE::testing::ErrnoSetterMatcher;
 using LlvmLibcMlockTest = LIBC_NAMESPACE::testing::ErrnoCheckingTest;

libc/test/src/sys/mman/linux/mremap_test.cpp

@@ -13,8 +13,6 @@
 #include "test/UnitTest/ErrnoSetterMatcher.h"
 #include "test/UnitTest/Test.h"
 
-#include <sys/mman.h>
-
 using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Fails;
 using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Succeeds;
 using LlvmLibcMremapTest = LIBC_NAMESPACE::testing::ErrnoCheckingTest;

libc/test/src/sys/mman/linux/shm_test.cpp

@@ -7,7 +7,6 @@
 //===----------------------------------------------------------------------===//
 
 #include "hdr/fcntl_macros.h"
-#include "src/__support/OSUtil/syscall.h"
 #include "src/fcntl/fcntl.h"
 #include "src/sys/mman/mmap.h"
 #include "src/sys/mman/munmap.h"
@@ -18,7 +17,6 @@
 #include "test/UnitTest/ErrnoCheckingTest.h"
 #include "test/UnitTest/ErrnoSetterMatcher.h"
 #include "test/UnitTest/Test.h"
-#include <sys/syscall.h>
 
 using namespace LIBC_NAMESPACE::testing::ErrnoSetterMatcher;
 using LlvmLibcShmTest = LIBC_NAMESPACE::testing::ErrnoCheckingTest;