[libc-commits] [libc] [llvm] [libc] implement popen/pclose (PR #232846)

[Pavel Labath via libc-commits]

================
@@ -0,0 +1,131 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+/// Implementation of the POSIX popen function for Linux.
+///
+//===----------------------------------------------------------------------===//
+
+#include "src/stdio/popen.h"
+
+#include "hdr/errno_macros.h"
+#include "hdr/fcntl_macros.h"
+#include "hdr/types/FILE.h"
+#include "hdr/types/pid_t.h"
+#include "hdr/unistd_macros.h"
+#include "src/__support/File/file.h"
+#include "src/__support/File/file_mode.h"
+#include "src/__support/File/linux/file.h"
+#include "src/__support/File/linux/popen_file.h"
+#include "src/__support/OSUtil/linux/shell_fork.h"
+#include "src/__support/OSUtil/linux/syscall_wrappers/close.h"
+#include "src/__support/OSUtil/linux/syscall_wrappers/dup2.h"
+#include "src/__support/OSUtil/linux/syscall_wrappers/fcntl.h"
+#include "src/__support/OSUtil/linux/syscall_wrappers/pipe2.h"
+#include "src/__support/common.h"
+#include "src/__support/error_or.h"
+#include "src/__support/libc_errno.h"
+#include "src/__support/macros/config.h"
+
+namespace LIBC_NAMESPACE_DECL {
+
+namespace {
+
+// A pipe only goes one way, so these are the only modes popen accepts.
+constexpr FileMode VALID_POPEN_MODES[] = {FileMode("r"), FileMode("w"),
+                                          FileMode("re"), FileMode("we")};
+
+// TODO: Simplify this with a cpp::find (or similar) helper in
+// src/__support/CPP/algorithm.h once one exists.
+bool is_valid_popen_mode(const FileMode &file_mode) {
+  for (const FileMode &valid_mode : VALID_POPEN_MODES)
+    if (file_mode == valid_mode)
+      return true;
+  return false;
+}
+
+// Clears FD_CLOEXEC on |fd|, leaving its other descriptor flags unchanged.
+ErrorOr<int> clear_close_on_exec(int fd) {
+  ErrorOr<int> flags = linux_syscalls::fcntl(fd, F_GETFD);
+  if (!flags)
+    return flags;
+  return linux_syscalls::fcntl(
+      fd, F_SETFD, reinterpret_cast<void *>(flags.value() & ~FD_CLOEXEC));
+}
+
+// Makes |fd| available as |target_fd| in a process that is about to exec.
+// Both ends of the pipe are created close-on-exec, so if |fd| is already
+// |target_fd| its FD_CLOEXEC flag has to be cleared instead.
+bool move_fd_for_exec(int fd, int target_fd) {
+  if (fd == target_fd)
+    return clear_close_on_exec(fd).has_value();
+  return linux_syscalls::dup2(fd, target_fd).has_value();
+}
+
+} // namespace
+
+LLVM_LIBC_FUNCTION(::FILE *, popen, (const char *command, const char *mode)) {
+  const FileMode file_mode(mode);
+  if (!is_valid_popen_mode(file_mode)) {
+    libc_errno = EINVAL;
+    return nullptr;
+  }
+  const bool read = file_mode.is_read();
+
+  // Both ends start out close-on-exec so that no other child process can
+  // inherit them. The child moves its end onto stdin or stdout before exec.
+  int pipe_fds[2];
+  if (ErrorOr<int> res = linux_syscalls::pipe2(pipe_fds, O_CLOEXEC); !res) {
+    libc_errno = res.error();
+    return nullptr;
+  }
+  const int parent_fd = read ? pipe_fds[0] : pipe_fds[1];
+  const int child_fd = read ? pipe_fds[1] : pipe_fds[0];
+  const int child_target_fd = read ? STDOUT_FILENO : STDIN_FILENO;
----------------
labath wrote:

Sounds good. FWIW, I think this looks much better than having it all caps. I'd change the rules to be:
- constexpr variables MUST be all caps (this should be easy to enforce)
- variables that CAN be constexpr, MUST be constexpr (I don't know if clang-tidy can do that)

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