[libc-commits] [libc] [libc] Introduce the ioctl syscall wrapper and port all callers (PR #204640)

[via libc-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-libc

Author: Pavel Labath (labath)

<details>
<summary>Changes</summary>

This patch adds an ioctl syscall wrapper in linux_syscalls namespace and migrates all direct SYS_ioctl calls to use it.

To handle the polymorphic nature of ioctl arguments (where some commands expect pointers, some expect scalar integers like queue_selector, and some expect no argument at all), I use a helper struct IoctlArg with implicit constructors. This avoids template bloat and overload ambiguities (particularly around literal 0) while keeping call sites clean.

Assisted by Gemini.

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


14 Files Affected:

- (modified) libc/src/__support/OSUtil/linux/syscall_wrappers/CMakeLists.txt (+13) 
- (added) libc/src/__support/OSUtil/linux/syscall_wrappers/ioctl.h (+52) 
- (modified) libc/src/sys/ioctl/linux/CMakeLists.txt (+1-2) 
- (modified) libc/src/sys/ioctl/linux/ioctl.cpp (+6-9) 
- (modified) libc/src/termios/linux/CMakeLists.txt (+7-14) 
- (modified) libc/src/termios/linux/tcdrain.cpp (+4-5) 
- (modified) libc/src/termios/linux/tcflow.cpp (+4-5) 
- (modified) libc/src/termios/linux/tcflush.cpp (+4-6) 
- (modified) libc/src/termios/linux/tcgetattr.cpp (+5-7) 
- (modified) libc/src/termios/linux/tcgetsid.cpp (+4-5) 
- (modified) libc/src/termios/linux/tcsendbreak.cpp (+4-5) 
- (modified) libc/src/termios/linux/tcsetattr.cpp (+5-7) 
- (modified) libc/src/unistd/linux/CMakeLists.txt (+2-3) 
- (modified) libc/src/unistd/linux/isatty.cpp (+5-8) 


``````````diff
diff --git a/libc/src/__support/OSUtil/linux/syscall_wrappers/CMakeLists.txt b/libc/src/__support/OSUtil/linux/syscall_wrappers/CMakeLists.txt
index a52be9676a3ca..f3282c315d9a9 100644
--- a/libc/src/__support/OSUtil/linux/syscall_wrappers/CMakeLists.txt
+++ b/libc/src/__support/OSUtil/linux/syscall_wrappers/CMakeLists.txt
@@ -706,6 +706,19 @@ add_header_library(
     libc.include.sys_syscall
 )
 
+add_header_library(
+  ioctl
+  HDRS
+    ioctl.h
+  DEPENDS
+    libc.src.__support.CPP.type_traits
+    libc.src.__support.OSUtil.osutil
+    libc.src.__support.error_or
+    libc.src.__support.macros.attributes
+    libc.src.__support.macros.config
+    libc.include.sys_syscall
+)
+
 add_header_library(
   readlink
   HDRS
diff --git a/libc/src/__support/OSUtil/linux/syscall_wrappers/ioctl.h b/libc/src/__support/OSUtil/linux/syscall_wrappers/ioctl.h
new file mode 100644
index 0000000000000..710e9af83961b
--- /dev/null
+++ b/libc/src/__support/OSUtil/linux/syscall_wrappers/ioctl.h
@@ -0,0 +1,52 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+/// Syscall wrapper for ioctl.
+///
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIBC_SRC___SUPPORT_OSUTIL_SYSCALL_WRAPPERS_IOCTL_H
+#define LLVM_LIBC_SRC___SUPPORT_OSUTIL_SYSCALL_WRAPPERS_IOCTL_H
+
+#include "src/__support/CPP/type_traits/enable_if.h"
+#include "src/__support/CPP/type_traits/is_integral.h"
+#include "src/__support/CPP/type_traits/is_null_pointer.h"
+#include "src/__support/CPP/type_traits/is_pointer.h"
+#include "src/__support/OSUtil/linux/syscall.h" // syscall_checked
+#include "src/__support/error_or.h"
+#include "src/__support/macros/attributes.h" // LIBC_INLINE
+#include "src/__support/macros/config.h"
+#include <sys/syscall.h> // For syscall numbers
+
+namespace LIBC_NAMESPACE_DECL {
+namespace linux_syscalls {
+
+struct IoctlArg {
+  unsigned long val;
+
+  LIBC_INLINE constexpr IoctlArg() : val(0) {}
+  LIBC_INLINE constexpr IoctlArg(cpp::nullptr_t) : val(0) {}
+
+  template <typename T, cpp::enable_if_t<cpp::is_pointer_v<T>, int> = 0>
+  LIBC_INLINE IoctlArg(T ptr) : val(reinterpret_cast<unsigned long>(ptr)) {}
+
+  template <typename T, cpp::enable_if_t<cpp::is_integral_v<T>, int> = 0>
+  LIBC_INLINE constexpr IoctlArg(T num)
+      : val(static_cast<unsigned long>(num)) {}
+};
+
+LIBC_INLINE ErrorOr<int> ioctl(int fd, unsigned long request,
+                               IoctlArg arg = {}) {
+  return syscall_checked<int>(SYS_ioctl, fd, request, arg.val);
+}
+
+} // namespace linux_syscalls
+} // namespace LIBC_NAMESPACE_DECL
+
+#endif // LLVM_LIBC_SRC___SUPPORT_OSUTIL_SYSCALL_WRAPPERS_IOCTL_H
diff --git a/libc/src/sys/ioctl/linux/CMakeLists.txt b/libc/src/sys/ioctl/linux/CMakeLists.txt
index 876f35aaee66c..33ff2d4dce214 100644
--- a/libc/src/sys/ioctl/linux/CMakeLists.txt
+++ b/libc/src/sys/ioctl/linux/CMakeLists.txt
@@ -6,7 +6,6 @@ add_entrypoint_object(
     ../ioctl.h
   DEPENDS
     libc.include.sys_ioctl
-    libc.include.sys_syscall
-    libc.src.__support.OSUtil.osutil
+    libc.src.__support.OSUtil.linux.syscall_wrappers.ioctl
     libc.src.errno.errno
 )
diff --git a/libc/src/sys/ioctl/linux/ioctl.cpp b/libc/src/sys/ioctl/linux/ioctl.cpp
index 9bb669c6a6f66..ec861ffc03ec6 100644
--- a/libc/src/sys/ioctl/linux/ioctl.cpp
+++ b/libc/src/sys/ioctl/linux/ioctl.cpp
@@ -8,11 +8,10 @@
 
 #include "src/sys/ioctl/ioctl.h"
 
-#include "src/__support/OSUtil/syscall.h" // For internal syscall function.
+#include "src/__support/OSUtil/linux/syscall_wrappers/ioctl.h"
 #include "src/__support/common.h"
 #include "src/__support/libc_errno.h"
 #include <stdarg.h>
-#include <sys/syscall.h> // For syscall numbers.
 
 namespace LIBC_NAMESPACE_DECL {
 
@@ -20,16 +19,14 @@ LLVM_LIBC_FUNCTION(int, ioctl, (int fd, unsigned long request, ...)) {
   va_list vargs;
   va_start(vargs, request);
   void *data_pointer = va_arg(vargs, void *);
-  int ret =
-      LIBC_NAMESPACE::syscall_impl<int>(SYS_ioctl, fd, request, data_pointer);
   va_end(vargs);
 
-  // Some ioctls can be expected to return positive values
-  if (ret >= 0)
-    return ret;
+  auto ret = linux_syscalls::ioctl(fd, request, data_pointer);
 
-  // If there is an error, errno is set and -1 is returned.
-  libc_errno = -ret;
+  if (ret.has_value())
+    return ret.value();
+
+  libc_errno = ret.error();
   return -1;
 }
 
diff --git a/libc/src/termios/linux/CMakeLists.txt b/libc/src/termios/linux/CMakeLists.txt
index e990fba25eabe..5d5440ae69266 100644
--- a/libc/src/termios/linux/CMakeLists.txt
+++ b/libc/src/termios/linux/CMakeLists.txt
@@ -51,9 +51,8 @@ add_entrypoint_object(
   HDRS
     ../tcgetsid.h
   DEPENDS
-    libc.include.sys_syscall
     libc.include.termios
-    libc.src.__support.OSUtil.osutil
+    libc.src.__support.OSUtil.linux.syscall_wrappers.ioctl
     libc.src.errno.errno
 )
 
@@ -64,9 +63,8 @@ add_entrypoint_object(
   HDRS
     ../tcdrain.h
   DEPENDS
-    libc.include.sys_syscall
     libc.include.termios
-    libc.src.__support.OSUtil.osutil
+    libc.src.__support.OSUtil.linux.syscall_wrappers.ioctl
     libc.src.errno.errno
 )
 
@@ -77,9 +75,8 @@ add_entrypoint_object(
   HDRS
     ../tcflush.h
   DEPENDS
-    libc.include.sys_syscall
     libc.include.termios
-    libc.src.__support.OSUtil.osutil
+    libc.src.__support.OSUtil.linux.syscall_wrappers.ioctl
     libc.src.errno.errno
 )
 
@@ -90,9 +87,8 @@ add_entrypoint_object(
   HDRS
     ../tcflow.h
   DEPENDS
-    libc.include.sys_syscall
     libc.include.termios
-    libc.src.__support.OSUtil.osutil
+    libc.src.__support.OSUtil.linux.syscall_wrappers.ioctl
     libc.src.errno.errno
 )
 
@@ -103,9 +99,8 @@ add_entrypoint_object(
   HDRS
     ../tcsendbreak.h
   DEPENDS
-    libc.include.sys_syscall
     libc.include.termios
-    libc.src.__support.OSUtil.osutil
+    libc.src.__support.OSUtil.linux.syscall_wrappers.ioctl
     libc.src.errno.errno
 )
 
@@ -123,9 +118,8 @@ add_entrypoint_object(
     ../tcgetattr.h
   DEPENDS
     .kernel_termios
-    libc.include.sys_syscall
     libc.include.termios
-    libc.src.__support.OSUtil.osutil
+    libc.src.__support.OSUtil.linux.syscall_wrappers.ioctl
     libc.src.errno.errno
 )
 
@@ -137,8 +131,7 @@ add_entrypoint_object(
     ../tcsetattr.h
   DEPENDS
     .kernel_termios
-    libc.include.sys_syscall
     libc.include.termios
-    libc.src.__support.OSUtil.osutil
+    libc.src.__support.OSUtil.linux.syscall_wrappers.ioctl
     libc.src.errno.errno
 )
diff --git a/libc/src/termios/linux/tcdrain.cpp b/libc/src/termios/linux/tcdrain.cpp
index 570b15c24fe7f..4fce89d65a76f 100644
--- a/libc/src/termios/linux/tcdrain.cpp
+++ b/libc/src/termios/linux/tcdrain.cpp
@@ -8,21 +8,20 @@
 
 #include "src/termios/tcdrain.h"
 
-#include "src/__support/OSUtil/syscall.h"
+#include "src/__support/OSUtil/linux/syscall_wrappers/ioctl.h"
 #include "src/__support/common.h"
 #include "src/__support/libc_errno.h"
 #include "src/__support/macros/config.h"
 
 #include <asm/ioctls.h> // Safe to include without the risk of name pollution.
-#include <sys/syscall.h> // For syscall numbers
 #include <termios.h>
 
 namespace LIBC_NAMESPACE_DECL {
 
 LLVM_LIBC_FUNCTION(int, tcdrain, (int fd)) {
-  int ret = LIBC_NAMESPACE::syscall_impl<int>(SYS_ioctl, fd, TCSBRK, 1);
-  if (ret < 0) {
-    libc_errno = -ret;
+  auto ret = linux_syscalls::ioctl(fd, TCSBRK, 1);
+  if (!ret.has_value()) {
+    libc_errno = ret.error();
     return -1;
   }
   return 0;
diff --git a/libc/src/termios/linux/tcflow.cpp b/libc/src/termios/linux/tcflow.cpp
index 714ef6aa71298..4ffd294997ad4 100644
--- a/libc/src/termios/linux/tcflow.cpp
+++ b/libc/src/termios/linux/tcflow.cpp
@@ -8,21 +8,20 @@
 
 #include "src/termios/tcflow.h"
 
-#include "src/__support/OSUtil/syscall.h"
+#include "src/__support/OSUtil/linux/syscall_wrappers/ioctl.h"
 #include "src/__support/common.h"
 #include "src/__support/libc_errno.h"
 #include "src/__support/macros/config.h"
 
 #include <asm/ioctls.h> // Safe to include without the risk of name pollution.
-#include <sys/syscall.h> // For syscall numbers
 #include <termios.h>
 
 namespace LIBC_NAMESPACE_DECL {
 
 LLVM_LIBC_FUNCTION(int, tcflow, (int fd, int action)) {
-  int ret = LIBC_NAMESPACE::syscall_impl<int>(SYS_ioctl, fd, TCXONC, action);
-  if (ret < 0) {
-    libc_errno = -ret;
+  auto ret = linux_syscalls::ioctl(fd, TCXONC, action);
+  if (!ret.has_value()) {
+    libc_errno = ret.error();
     return -1;
   }
   return 0;
diff --git a/libc/src/termios/linux/tcflush.cpp b/libc/src/termios/linux/tcflush.cpp
index 4c7b9fadc446d..8a4676d97454a 100644
--- a/libc/src/termios/linux/tcflush.cpp
+++ b/libc/src/termios/linux/tcflush.cpp
@@ -8,22 +8,20 @@
 
 #include "src/termios/tcflush.h"
 
-#include "src/__support/OSUtil/syscall.h"
+#include "src/__support/OSUtil/linux/syscall_wrappers/ioctl.h"
 #include "src/__support/common.h"
 #include "src/__support/libc_errno.h"
 #include "src/__support/macros/config.h"
 
 #include <asm/ioctls.h> // Safe to include without the risk of name pollution.
-#include <sys/syscall.h> // For syscall numbers
 #include <termios.h>
 
 namespace LIBC_NAMESPACE_DECL {
 
 LLVM_LIBC_FUNCTION(int, tcflush, (int fd, int queue_selector)) {
-  int ret =
-      LIBC_NAMESPACE::syscall_impl<int>(SYS_ioctl, fd, TCFLSH, queue_selector);
-  if (ret < 0) {
-    libc_errno = -ret;
+  auto ret = linux_syscalls::ioctl(fd, TCFLSH, queue_selector);
+  if (!ret.has_value()) {
+    libc_errno = ret.error();
     return -1;
   }
   return 0;
diff --git a/libc/src/termios/linux/tcgetattr.cpp b/libc/src/termios/linux/tcgetattr.cpp
index 2e768269c874d..0569be4ae588f 100644
--- a/libc/src/termios/linux/tcgetattr.cpp
+++ b/libc/src/termios/linux/tcgetattr.cpp
@@ -7,24 +7,22 @@
 //===----------------------------------------------------------------------===//
 
 #include "src/termios/tcgetattr.h"
-#include "kernel_termios.h"
-
-#include "src/__support/OSUtil/syscall.h"
+#include "src/__support/OSUtil/linux/syscall_wrappers/ioctl.h"
 #include "src/__support/common.h"
 #include "src/__support/libc_errno.h"
 #include "src/__support/macros/config.h"
+#include "src/termios/linux/kernel_termios.h"
 
 #include <asm/ioctls.h> // Safe to include without the risk of name pollution.
-#include <sys/syscall.h> // For syscall numbers
 #include <termios.h>
 
 namespace LIBC_NAMESPACE_DECL {
 
 LLVM_LIBC_FUNCTION(int, tcgetattr, (int fd, struct termios *t)) {
   LIBC_NAMESPACE::kernel_termios kt;
-  int ret = LIBC_NAMESPACE::syscall_impl<int>(SYS_ioctl, fd, TCGETS, &kt);
-  if (ret < 0) {
-    libc_errno = -ret;
+  auto ret = linux_syscalls::ioctl(fd, TCGETS, &kt);
+  if (!ret.has_value()) {
+    libc_errno = ret.error();
     return -1;
   }
   t->c_iflag = kt.c_iflag;
diff --git a/libc/src/termios/linux/tcgetsid.cpp b/libc/src/termios/linux/tcgetsid.cpp
index 7487816cf2741..428ca83fb2ad0 100644
--- a/libc/src/termios/linux/tcgetsid.cpp
+++ b/libc/src/termios/linux/tcgetsid.cpp
@@ -8,22 +8,21 @@
 
 #include "src/termios/tcgetsid.h"
 
-#include "src/__support/OSUtil/syscall.h"
+#include "src/__support/OSUtil/linux/syscall_wrappers/ioctl.h"
 #include "src/__support/common.h"
 #include "src/__support/libc_errno.h"
 #include "src/__support/macros/config.h"
 
 #include <asm/ioctls.h> // Safe to include without the risk of name pollution.
-#include <sys/syscall.h> // For syscall numbers
 #include <termios.h>
 
 namespace LIBC_NAMESPACE_DECL {
 
 LLVM_LIBC_FUNCTION(pid_t, tcgetsid, (int fd)) {
   pid_t sid;
-  int ret = LIBC_NAMESPACE::syscall_impl<int>(SYS_ioctl, fd, TIOCGSID, &sid);
-  if (ret < 0) {
-    libc_errno = -ret;
+  auto ret = linux_syscalls::ioctl(fd, TIOCGSID, &sid);
+  if (!ret.has_value()) {
+    libc_errno = ret.error();
     return -1;
   }
   return sid;
diff --git a/libc/src/termios/linux/tcsendbreak.cpp b/libc/src/termios/linux/tcsendbreak.cpp
index 1d546c1d5953e..e91ec7b748582 100644
--- a/libc/src/termios/linux/tcsendbreak.cpp
+++ b/libc/src/termios/linux/tcsendbreak.cpp
@@ -8,13 +8,12 @@
 
 #include "src/termios/tcsendbreak.h"
 
-#include "src/__support/OSUtil/syscall.h"
+#include "src/__support/OSUtil/linux/syscall_wrappers/ioctl.h"
 #include "src/__support/common.h"
 #include "src/__support/libc_errno.h"
 #include "src/__support/macros/config.h"
 
 #include <asm/ioctls.h> // Safe to include without the risk of name pollution.
-#include <sys/syscall.h> // For syscall numbers
 #include <termios.h>
 
 namespace LIBC_NAMESPACE_DECL {
@@ -23,9 +22,9 @@ LLVM_LIBC_FUNCTION(pid_t, tcsendbreak, (int fd, int /* unused duration */)) {
   // POSIX leaves the behavior for non-zero duration implementation dependent.
   // Which means that the behavior can be the same as it is when duration is
   // zero. So, we just pass zero to the syscall.
-  int ret = LIBC_NAMESPACE::syscall_impl<int>(SYS_ioctl, fd, TCSBRK, 0);
-  if (ret < 0) {
-    libc_errno = -ret;
+  auto ret = linux_syscalls::ioctl(fd, TCSBRK, 0);
+  if (!ret.has_value()) {
+    libc_errno = ret.error();
     return -1;
   }
   return 0;
diff --git a/libc/src/termios/linux/tcsetattr.cpp b/libc/src/termios/linux/tcsetattr.cpp
index 8a2c7290217ba..b2f08d078ee83 100644
--- a/libc/src/termios/linux/tcsetattr.cpp
+++ b/libc/src/termios/linux/tcsetattr.cpp
@@ -7,15 +7,13 @@
 //===----------------------------------------------------------------------===//
 
 #include "src/termios/tcsetattr.h"
-#include "kernel_termios.h"
-
-#include "src/__support/OSUtil/syscall.h"
+#include "src/__support/OSUtil/linux/syscall_wrappers/ioctl.h"
 #include "src/__support/common.h"
 #include "src/__support/libc_errno.h"
 #include "src/__support/macros/config.h"
+#include "src/termios/linux/kernel_termios.h"
 
 #include <asm/ioctls.h> // Safe to include without the risk of name pollution.
-#include <sys/syscall.h> // For syscall numbers
 #include <termios.h>
 
 namespace LIBC_NAMESPACE_DECL {
@@ -52,9 +50,9 @@ LLVM_LIBC_FUNCTION(int, tcsetattr,
       kt.c_cc[i] = 0;
   }
 
-  int ret = LIBC_NAMESPACE::syscall_impl<int>(SYS_ioctl, fd, cmd, &kt);
-  if (ret < 0) {
-    libc_errno = -ret;
+  auto ret = linux_syscalls::ioctl(fd, cmd, &kt);
+  if (!ret.has_value()) {
+    libc_errno = ret.error();
     return -1;
   }
   return 0;
diff --git a/libc/src/unistd/linux/CMakeLists.txt b/libc/src/unistd/linux/CMakeLists.txt
index 04ccde414cd2f..af385e9bbed72 100644
--- a/libc/src/unistd/linux/CMakeLists.txt
+++ b/libc/src/unistd/linux/CMakeLists.txt
@@ -342,11 +342,10 @@ add_entrypoint_object(
   HDRS
     ../isatty.h
   DEPENDS
-    libc.hdr.fcntl_macros
+    libc.hdr.unistd_macros
     libc.include.unistd
     libc.include.sys_ioctl
-    libc.include.sys_syscall
-    libc.src.__support.OSUtil.osutil
+    libc.src.__support.OSUtil.linux.syscall_wrappers.ioctl
     libc.src.errno.errno
 )
 
diff --git a/libc/src/unistd/linux/isatty.cpp b/libc/src/unistd/linux/isatty.cpp
index a4d17912b57b0..4418feb2229a3 100644
--- a/libc/src/unistd/linux/isatty.cpp
+++ b/libc/src/unistd/linux/isatty.cpp
@@ -8,13 +8,11 @@
 
 #include "src/unistd/isatty.h"
 
-#include "src/__support/OSUtil/syscall.h" // For internal syscall function.
+#include "src/__support/OSUtil/linux/syscall_wrappers/ioctl.h"
 #include "src/__support/common.h"
-
 #include "src/__support/libc_errno.h"
 #include "src/__support/macros/config.h"
-#include <sys/ioctl.h>   // For ioctl numbers.
-#include <sys/syscall.h> // For syscall numbers.
+#include <sys/ioctl.h> // For ioctl numbers.
 
 namespace LIBC_NAMESPACE_DECL {
 
@@ -23,12 +21,11 @@ LLVM_LIBC_FUNCTION(int, isatty, (int fd)) {
   int line_d_val = INIT_VAL;
   // This gets the line dicipline of the terminal. When called on something that
   // isn't a terminal it doesn't change line_d_val and returns -1.
-  int result =
-      LIBC_NAMESPACE::syscall_impl<int>(SYS_ioctl, fd, TIOCGETD, &line_d_val);
-  if (result == 0)
+  auto result = linux_syscalls::ioctl(fd, TIOCGETD, &line_d_val);
+  if (result.has_value())
     return 1;
 
-  libc_errno = -result;
+  libc_errno = result.error();
   return 0;
 }
 

``````````

</details>


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