[libc-commits] [libc] [libc] Use linux_syscalls::open instead of manual SYS_open ifdefs (PR #201089)

[via libc-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-libc

Author: Pavel Labath (labath)

<details>
<summary>Changes</summary>

I needed to clean up a few types in file.cpp in order to match the wrapper prototype.

Assisted by Gemini.

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


11 Files Affected:

- (modified) libc/src/__support/File/linux/CMakeLists.txt (+2) 
- (modified) libc/src/__support/File/linux/dir.cpp (+2-15) 
- (modified) libc/src/__support/File/linux/file.cpp (+8-16) 
- (modified) libc/src/__support/OSUtil/linux/CMakeLists.txt (+1) 
- (modified) libc/src/__support/OSUtil/linux/auxv.h (+8-10) 
- (modified) libc/src/__support/threads/linux/CMakeLists.txt (+1) 
- (modified) libc/src/__support/threads/linux/thread.cpp (+13-24) 
- (modified) libc/src/fcntl/linux/CMakeLists.txt (+1) 
- (modified) libc/src/fcntl/linux/creat.cpp (+6-12) 
- (modified) libc/src/spawn/linux/CMakeLists.txt (+1-1) 
- (modified) libc/src/spawn/linux/posix_spawn.cpp (+3-18) 


``````````diff
diff --git a/libc/src/__support/File/linux/CMakeLists.txt b/libc/src/__support/File/linux/CMakeLists.txt
index 2becb385e44d9..7b770f916a251 100644
--- a/libc/src/__support/File/linux/CMakeLists.txt
+++ b/libc/src/__support/File/linux/CMakeLists.txt
@@ -14,6 +14,7 @@ add_object_library(
     libc.src.__support.CPP.new
     libc.src.__support.OSUtil.osutil
     libc.src.__support.OSUtil.linux.syscall_wrappers.lseek
+    libc.src.__support.OSUtil.linux.syscall_wrappers.open
     libc.src.__support.error_or
     libc.src.__support.File.file
     libc.src.errno.errno
@@ -27,6 +28,7 @@ add_object_library(
     libc.hdr.fcntl_macros
     libc.include.sys_syscall
     libc.src.__support.OSUtil.osutil
+    libc.src.__support.OSUtil.linux.syscall_wrappers.open
     libc.src.__support.error_or
     libc.src.errno.errno
     libc.src.__support.File.dir
diff --git a/libc/src/__support/File/linux/dir.cpp b/libc/src/__support/File/linux/dir.cpp
index 5fe44fa8297b6..8412b42348559 100644
--- a/libc/src/__support/File/linux/dir.cpp
+++ b/libc/src/__support/File/linux/dir.cpp
@@ -8,6 +8,7 @@
 
 #include "src/__support/File/dir.h"
 
+#include "src/__support/OSUtil/linux/syscall_wrappers/open.h"
 #include "src/__support/OSUtil/syscall.h" // For internal syscall function.
 #include "src/__support/error_or.h"
 #include "src/__support/macros/config.h"
@@ -18,21 +19,7 @@
 namespace LIBC_NAMESPACE_DECL {
 
 ErrorOr<int> platform_opendir(const char *name) {
-  int open_flags = O_RDONLY | O_DIRECTORY | O_CLOEXEC;
-#ifdef SYS_open
-  int fd = LIBC_NAMESPACE::syscall_impl<int>(SYS_open, name, open_flags);
-#elif defined(SYS_openat)
-  int fd =
-      LIBC_NAMESPACE::syscall_impl<int>(SYS_openat, AT_FDCWD, name, open_flags);
-#else
-#error                                                                         \
-    "SYS_open and SYS_openat syscalls not available to perform an open operation."
-#endif
-
-  if (fd < 0) {
-    return LIBC_NAMESPACE::Error(-fd);
-  }
-  return fd;
+  return linux_syscalls::open(name, O_RDONLY | O_DIRECTORY | O_CLOEXEC, 0);
 }
 
 ErrorOr<size_t> platform_fetch_dirents(int fd, cpp::span<uint8_t> buffer) {
diff --git a/libc/src/__support/File/linux/file.cpp b/libc/src/__support/File/linux/file.cpp
index 2bef96a102a0c..e89b3d118deb1 100644
--- a/libc/src/__support/File/linux/file.cpp
+++ b/libc/src/__support/File/linux/file.cpp
@@ -14,6 +14,7 @@
 #include "src/__support/File/file.h"
 #include "src/__support/OSUtil/fcntl.h"
 #include "src/__support/OSUtil/linux/syscall_wrappers/lseek.h"
+#include "src/__support/OSUtil/linux/syscall_wrappers/open.h"
 #include "src/__support/OSUtil/syscall.h" // For internal syscall function.
 #include "src/__support/alloc-checker.h"
 #include "src/__support/libc_errno.h" // For error macros
@@ -68,7 +69,7 @@ ErrorOr<File *> openfile(const char *path, const char *mode) {
     // return {nullptr, EINVAL};
     return Error(EINVAL);
   }
-  long open_flags = 0;
+  int open_flags = 0;
   if (modeflags & ModeFlags(File::OpenMode::APPEND)) {
     open_flags = O_CREAT | O_APPEND;
     if (modeflags & ModeFlags(File::OpenMode::PLUS))
@@ -89,21 +90,12 @@ ErrorOr<File *> openfile(const char *path, const char *mode) {
   }
 
   // File created will have 0666 permissions.
-  constexpr long OPEN_MODE =
+  constexpr mode_t OPEN_MODE =
       S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH;
 
-#ifdef SYS_open
-  int fd =
-      LIBC_NAMESPACE::syscall_impl<int>(SYS_open, path, open_flags, OPEN_MODE);
-#elif defined(SYS_openat)
-  int fd = LIBC_NAMESPACE::syscall_impl<int>(SYS_openat, AT_FDCWD, path,
-                                             open_flags, OPEN_MODE);
-#else
-#error "open and openat syscalls not available."
-#endif
-
-  if (fd < 0)
-    return Error(-fd);
+  ErrorOr<int> fd = linux_syscalls::open(path, open_flags, OPEN_MODE);
+  if (!fd)
+    return Error(fd.error());
 
   uint8_t *buffer;
   {
@@ -113,8 +105,8 @@ ErrorOr<File *> openfile(const char *path, const char *mode) {
       return Error(ENOMEM);
   }
   AllocChecker ac;
-  auto *file = new (ac)
-      LinuxFile(fd, buffer, File::DEFAULT_BUFFER_SIZE, _IOFBF, true, modeflags);
+  auto *file = new (ac) LinuxFile(fd.value(), buffer, File::DEFAULT_BUFFER_SIZE,
+                                  _IOFBF, true, modeflags);
   if (!ac)
     return Error(ENOMEM);
   File::add_file(file);
diff --git a/libc/src/__support/OSUtil/linux/CMakeLists.txt b/libc/src/__support/OSUtil/linux/CMakeLists.txt
index 3350fb298a7fd..525e62c7ca23b 100644
--- a/libc/src/__support/OSUtil/linux/CMakeLists.txt
+++ b/libc/src/__support/OSUtil/linux/CMakeLists.txt
@@ -36,6 +36,7 @@ add_header_library(
     libc.hdr.sys_mman_macros
     libc.src.__support.OSUtil.linux.syscall_wrappers.mmap
     libc.src.__support.OSUtil.linux.syscall_wrappers.munmap
+    libc.src.__support.OSUtil.linux.syscall_wrappers.open
     libc.src.__support.OSUtil.osutil
     libc.src.__support.common
     libc.src.__support.CPP.optional
diff --git a/libc/src/__support/OSUtil/linux/auxv.h b/libc/src/__support/OSUtil/linux/auxv.h
index 972ad78276f21..b4c990f9721bc 100644
--- a/libc/src/__support/OSUtil/linux/auxv.h
+++ b/libc/src/__support/OSUtil/linux/auxv.h
@@ -9,11 +9,12 @@
 #ifndef LLVM_LIBC_SRC___SUPPORT_OSUTIL_LINUX_AUXV_H
 #define LLVM_LIBC_SRC___SUPPORT_OSUTIL_LINUX_AUXV_H
 
-#include "hdr/fcntl_macros.h" // For open flags
+#include "hdr/fcntl_macros.h"    // For open flags
 #include "hdr/sys_auxv_macros.h" // For AT_ macros
 #include "hdr/sys_mman_macros.h" // For mmap flags
 #include "src/__support/OSUtil/linux/syscall_wrappers/mmap.h"
 #include "src/__support/OSUtil/linux/syscall_wrappers/munmap.h"
+#include "src/__support/OSUtil/linux/syscall_wrappers/open.h"
 #include "src/__support/OSUtil/syscall.h"
 #include "src/__support/common.h"
 #include "src/__support/threads/callonce.h"
@@ -112,20 +113,17 @@ LIBC_INLINE void Vector::fallback_initialize_unsync() {
 #endif
 
   // Attempt 2: read /proc/self/auxv.
-#ifdef SYS_openat
-  int fd = syscall_impl<int>(SYS_openat, AT_FDCWD, "/proc/self/auxv",
-                             O_RDONLY | O_CLOEXEC);
-#else
-  int fd = syscall_impl<int>(SYS_open, "/proc/self/auxv", O_RDONLY | O_CLOEXEC);
-#endif
-  if (fd < 0) {
+  ErrorOr<int> fd =
+      linux_syscalls::open("/proc/self/auxv", O_RDONLY | O_CLOEXEC, 0);
+  if (!fd) {
     linux_syscalls::munmap(vector, AUXV_MMAP_SIZE);
     return;
   }
   uint8_t *cursor = reinterpret_cast<uint8_t *>(vector);
   bool has_error = false;
   while (avaiable_size != 0) {
-    long bytes_read = syscall_impl<long>(SYS_read, fd, cursor, avaiable_size);
+    long bytes_read =
+        syscall_impl<long>(SYS_read, fd.value(), cursor, avaiable_size);
     if (bytes_read <= 0) {
       if (bytes_read == -EINTR)
         continue;
@@ -135,7 +133,7 @@ LIBC_INLINE void Vector::fallback_initialize_unsync() {
     avaiable_size -= bytes_read;
     cursor += bytes_read;
   }
-  syscall_impl<long>(SYS_close, fd);
+  syscall_impl<long>(SYS_close, fd.value());
   if (has_error) {
     linux_syscalls::munmap(vector, AUXV_MMAP_SIZE);
     return;
diff --git a/libc/src/__support/threads/linux/CMakeLists.txt b/libc/src/__support/threads/linux/CMakeLists.txt
index ad898d5c79514..8344c6c0462a3 100644
--- a/libc/src/__support/threads/linux/CMakeLists.txt
+++ b/libc/src/__support/threads/linux/CMakeLists.txt
@@ -44,6 +44,7 @@ add_object_library(
     libc.src.__support.OSUtil.linux.syscall_wrappers.mmap
     libc.src.__support.OSUtil.linux.syscall_wrappers.mprotect
     libc.src.__support.OSUtil.linux.syscall_wrappers.munmap
+    libc.src.__support.OSUtil.linux.syscall_wrappers.open
     libc.src.__support.threads.thread_common
   COMPILE_OPTIONS
     ${libc_opt_high_flag}
diff --git a/libc/src/__support/threads/linux/thread.cpp b/libc/src/__support/threads/linux/thread.cpp
index 28b5fa0fd4dab..64e043862ff74 100644
--- a/libc/src/__support/threads/linux/thread.cpp
+++ b/libc/src/__support/threads/linux/thread.cpp
@@ -14,6 +14,7 @@
 #include "src/__support/OSUtil/linux/syscall_wrappers/mmap.h"
 #include "src/__support/OSUtil/linux/syscall_wrappers/mprotect.h"
 #include "src/__support/OSUtil/linux/syscall_wrappers/munmap.h"
+#include "src/__support/OSUtil/linux/syscall_wrappers/open.h"
 #include "src/__support/OSUtil/syscall.h" // For syscall functions.
 #include "src/__support/common.h"
 #include "src/__support/error_or.h"
@@ -430,19 +431,13 @@ int Thread::set_name(const cpp::string_view &name) {
   char path_name_buffer[THREAD_NAME_PATH_SIZE];
   cpp::StringStream path_stream(path_name_buffer);
   construct_thread_name_file_path(path_stream, attrib->tid);
-#ifdef SYS_open
-  int fd =
-      LIBC_NAMESPACE::syscall_impl<int>(SYS_open, path_name_buffer, O_RDWR);
-#else
-  int fd = LIBC_NAMESPACE::syscall_impl<int>(SYS_openat, AT_FDCWD,
-                                             path_name_buffer, O_RDWR);
-#endif
-  if (fd < 0)
-    return -fd;
+  ErrorOr<int> fd = linux_syscalls::open(path_name_buffer, O_RDWR, 0);
+  if (!fd)
+    return fd.error();
 
-  int retval = LIBC_NAMESPACE::syscall_impl<int>(SYS_write, fd, name.data(),
-                                                 name.size());
-  LIBC_NAMESPACE::syscall_impl<long>(SYS_close, fd);
+  int retval = LIBC_NAMESPACE::syscall_impl<int>(SYS_write, fd.value(),
+                                                 name.data(), name.size());
+  LIBC_NAMESPACE::syscall_impl<long>(SYS_close, fd.value());
 
   if (retval < 0)
     return -retval;
@@ -472,19 +467,13 @@ int Thread::get_name(cpp::StringStream &name) const {
   char path_name_buffer[THREAD_NAME_PATH_SIZE];
   cpp::StringStream path_stream(path_name_buffer);
   construct_thread_name_file_path(path_stream, attrib->tid);
-#ifdef SYS_open
-  int fd =
-      LIBC_NAMESPACE::syscall_impl<int>(SYS_open, path_name_buffer, O_RDONLY);
-#else
-  int fd = LIBC_NAMESPACE::syscall_impl<int>(SYS_openat, AT_FDCWD,
-                                             path_name_buffer, O_RDONLY);
-#endif
-  if (fd < 0)
-    return -fd;
+  ErrorOr<int> fd = linux_syscalls::open(path_name_buffer, O_RDONLY, 0);
+  if (!fd)
+    return fd.error();
 
-  int retval = LIBC_NAMESPACE::syscall_impl<int>(SYS_read, fd, name_buffer,
-                                                 NAME_SIZE_MAX);
-  LIBC_NAMESPACE::syscall_impl<long>(SYS_close, fd);
+  int retval = LIBC_NAMESPACE::syscall_impl<int>(SYS_read, fd.value(),
+                                                 name_buffer, NAME_SIZE_MAX);
+  LIBC_NAMESPACE::syscall_impl<long>(SYS_close, fd.value());
   if (retval < 0)
     return -retval;
   if (retval == NAME_SIZE_MAX)
diff --git a/libc/src/fcntl/linux/CMakeLists.txt b/libc/src/fcntl/linux/CMakeLists.txt
index 4c151edc0ab49..be5fd4fa8f44b 100644
--- a/libc/src/fcntl/linux/CMakeLists.txt
+++ b/libc/src/fcntl/linux/CMakeLists.txt
@@ -6,6 +6,7 @@ add_entrypoint_object(
     ../creat.h
   DEPENDS
     libc.hdr.fcntl_macros
+    libc.src.__support.OSUtil.linux.syscall_wrappers.open
     libc.src.__support.OSUtil.osutil
     libc.src.errno.errno
 )
diff --git a/libc/src/fcntl/linux/creat.cpp b/libc/src/fcntl/linux/creat.cpp
index e74cef299b59f..9dc65e99b0e54 100644
--- a/libc/src/fcntl/linux/creat.cpp
+++ b/libc/src/fcntl/linux/creat.cpp
@@ -8,30 +8,24 @@
 
 #include "src/fcntl/creat.h"
 
-#include "src/__support/OSUtil/syscall.h" // For internal syscall function.
+#include "src/__support/OSUtil/linux/syscall_wrappers/open.h"
 #include "src/__support/common.h"
 #include "src/__support/libc_errno.h"
 #include "src/__support/macros/config.h"
 
 #include "hdr/fcntl_macros.h"
-#include <sys/syscall.h> // For syscall numbers.
 
 namespace LIBC_NAMESPACE_DECL {
 
 LLVM_LIBC_FUNCTION(int, creat, (const char *path, int mode_flags)) {
-#ifdef SYS_open
-  int fd = LIBC_NAMESPACE::syscall_impl<int>(
-      SYS_open, path, O_CREAT | O_WRONLY | O_TRUNC, mode_flags);
-#else
-  int fd = LIBC_NAMESPACE::syscall_impl<int>(
-      SYS_openat, AT_FDCWD, path, O_CREAT | O_WRONLY | O_TRUNC, mode_flags);
-#endif
+  ErrorOr<int> fd =
+      linux_syscalls::open(path, O_CREAT | O_WRONLY | O_TRUNC, mode_flags);
 
-  if (fd < 0) {
-    libc_errno = -fd;
+  if (!fd) {
+    libc_errno = fd.error();
     return -1;
   }
-  return fd;
+  return fd.value();
 }
 
 } // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/src/spawn/linux/CMakeLists.txt b/libc/src/spawn/linux/CMakeLists.txt
index a01125a4ae440..42057e9f12fec 100644
--- a/libc/src/spawn/linux/CMakeLists.txt
+++ b/libc/src/spawn/linux/CMakeLists.txt
@@ -10,8 +10,8 @@ add_entrypoint_object(
     libc.include.spawn
     libc.include.sys_syscall
     libc.include.signal
-    libc.src.__support.CPP.optional
     libc.src.__support.OSUtil.osutil
+    libc.src.__support.OSUtil.linux.syscall_wrappers.open
     libc.src.spawn.file_actions
     libc.src.signal.linux.signal_utils
 )
diff --git a/libc/src/spawn/linux/posix_spawn.cpp b/libc/src/spawn/linux/posix_spawn.cpp
index 1a4ec72ce0d00..301be6d7c8762 100644
--- a/libc/src/spawn/linux/posix_spawn.cpp
+++ b/libc/src/spawn/linux/posix_spawn.cpp
@@ -7,8 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "src/spawn/posix_spawn.h"
-
-#include "src/__support/CPP/optional.h"
+#include "src/__support/OSUtil/linux/syscall_wrappers/open.h"
 #include "src/__support/OSUtil/syscall.h" // For internal syscall function.
 #include "src/__support/common.h"
 #include "src/__support/macros/config.h"
@@ -40,21 +39,6 @@ pid_t fork() {
 #endif
 }
 
-cpp::optional<int> open(const char *path, int oflags, mode_t mode) {
-#ifdef SYS_open
-  int fd = LIBC_NAMESPACE::syscall_impl<int>(SYS_open, path, oflags, mode);
-#else
-  int fd = LIBC_NAMESPACE::syscall_impl<int>(SYS_openat, AT_FDCWD, path, oflags,
-                                             mode);
-#endif
-  if (fd >= 0)
-    return fd;
-  // The open function is called as part of the child process' preparatory
-  // steps. If an open fails, the child process just exits. So, unlike
-  // the public open function, we do not need to set errno here.
-  return cpp::nullopt;
-}
-
 void close(int fd) { LIBC_NAMESPACE::syscall_impl<long>(SYS_close, fd); }
 
 // We use dup3 if dup2 is not available, similar to our implementation of dup2
@@ -94,7 +78,8 @@ void child_process(const char *__restrict path,
       switch (act->type) {
       case BaseSpawnFileAction::OPEN: {
         auto *open_act = reinterpret_cast<SpawnFileOpenAction *>(act);
-        auto fd = open(open_act->path, open_act->oflag, open_act->mode);
+        ErrorOr<int> fd = linux_syscalls::open(open_act->path, open_act->oflag,
+                                               open_act->mode);
         if (!fd)
           exit();
         int actual_fd = *fd;

``````````

</details>


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