[libc-commits] [libc] [libc] Add the implementation of the fdopen function (PR #94186)

Xu Zhang via libc-commits libc-commits at lists.llvm.org
Fri Jun 7 12:42:16 PDT 2024


https://github.com/simonzgx updated https://github.com/llvm/llvm-project/pull/94186

>From 15834c5b7471987c1096a833990075b018f10847 Mon Sep 17 00:00:00 2001
From: Xu Zhang <simonzgx at gmail.com>
Date: Tue, 4 Jun 2024 01:54:59 +0800
Subject: [PATCH 1/9] [libc] Add the implementation of the fdopen function

---
 libc/config/linux/aarch64/entrypoints.txt |  1 +
 libc/config/linux/riscv/entrypoints.txt   |  1 +
 libc/config/linux/x86_64/entrypoints.txt  |  1 +
 libc/spec/posix.td                        |  5 ++
 libc/src/fcntl/linux/fcntl.cpp            | 13 ++--
 libc/src/stdio/CMakeLists.txt             |  7 ++
 libc/src/stdio/fdopen.h                   | 20 ++++++
 libc/src/stdio/linux/CMakeLists.txt       | 15 +++++
 libc/src/stdio/linux/fdopen.cpp           | 79 +++++++++++++++++++++++
 libc/test/src/fcntl/fcntl_test.cpp        | 10 +++
 libc/test/src/stdio/CMakeLists.txt        | 14 ++++
 libc/test/src/stdio/fdopen_test.cpp       | 68 +++++++++++++++++++
 12 files changed, 230 insertions(+), 4 deletions(-)
 create mode 100644 libc/src/stdio/fdopen.h
 create mode 100644 libc/src/stdio/linux/fdopen.cpp
 create mode 100644 libc/test/src/stdio/fdopen_test.cpp

diff --git a/libc/config/linux/aarch64/entrypoints.txt b/libc/config/linux/aarch64/entrypoints.txt
index 74a355407b4b9..d361cac55b3d9 100644
--- a/libc/config/linux/aarch64/entrypoints.txt
+++ b/libc/config/linux/aarch64/entrypoints.txt
@@ -201,6 +201,7 @@ set(TARGET_LIBC_ENTRYPOINTS
     libc.src.stdio.snprintf
     libc.src.stdio.vsprintf
     libc.src.stdio.vsnprintf
+    libc.src.stdio.fdopen
     #libc.src.stdio.sscanf
     #libc.src.stdio.scanf
     #libc.src.stdio.fscanf
diff --git a/libc/config/linux/riscv/entrypoints.txt b/libc/config/linux/riscv/entrypoints.txt
index 479af40b5b26b..fc5300f9be2d0 100644
--- a/libc/config/linux/riscv/entrypoints.txt
+++ b/libc/config/linux/riscv/entrypoints.txt
@@ -209,6 +209,7 @@ set(TARGET_LIBC_ENTRYPOINTS
     libc.src.stdio.sscanf
     libc.src.stdio.scanf
     libc.src.stdio.fscanf
+    libc.src.stdio.fdopen
 
     # sys/mman.h entrypoints
     libc.src.sys.mman.madvise
diff --git a/libc/config/linux/x86_64/entrypoints.txt b/libc/config/linux/x86_64/entrypoints.txt
index 9a87f3d6d5f00..2a2ceb8f18460 100644
--- a/libc/config/linux/x86_64/entrypoints.txt
+++ b/libc/config/linux/x86_64/entrypoints.txt
@@ -215,6 +215,7 @@ set(TARGET_LIBC_ENTRYPOINTS
     libc.src.stdio.scanf
     libc.src.stdio.fscanf
     libc.src.stdio.fileno
+    libc.src.stdio.fdopen
 
     # sys/epoll.h entrypoints
     libc.src.sys.epoll.epoll_create
diff --git a/libc/spec/posix.td b/libc/spec/posix.td
index e16353b8142de..78d9d727ac7b4 100644
--- a/libc/spec/posix.td
+++ b/libc/spec/posix.td
@@ -1293,6 +1293,11 @@ def POSIX : StandardSpec<"POSIX"> {
             RetValSpec<IntType>,
             [ArgSpec<FILEPtr>]
           >,
+          FunctionSpec<
+            "fdopen",
+            RetValSpec<FILEPtr>,
+            [ArgSpec<IntType>, ArgSpec<ConstCharPtr>]
+          >,
       ]
   >;
 
diff --git a/libc/src/fcntl/linux/fcntl.cpp b/libc/src/fcntl/linux/fcntl.cpp
index 24a20fb364109..4100ec4298a20 100644
--- a/libc/src/fcntl/linux/fcntl.cpp
+++ b/libc/src/fcntl/linux/fcntl.cpp
@@ -29,8 +29,6 @@ LLVM_LIBC_FUNCTION(int, fcntl, (int fd, int cmd, ...)) {
   va_end(varargs);
 
   switch (cmd) {
-  case F_SETLKW:
-    return syscall_impl<int>(SYS_fcntl, fd, cmd, arg);
   case F_OFD_SETLKW: {
     struct flock *flk = reinterpret_cast<struct flock *>(arg);
     // convert the struct to a flock64
@@ -86,8 +84,15 @@ LLVM_LIBC_FUNCTION(int, fcntl, (int fd, int cmd, ...)) {
     return -1;
   }
   // The general case
-  default:
-    return syscall_impl<int>(SYS_fcntl, fd, cmd, reinterpret_cast<void *>(arg));
+  default: {
+    int retVal =
+        syscall_impl<int>(SYS_fcntl, fd, cmd, reinterpret_cast<void *>(arg));
+    if (retVal >= 0) {
+      return retVal;
+    }
+    libc_errno = -retVal;
+    return -1;
+  }
   }
 }
 } // namespace LIBC_NAMESPACE
diff --git a/libc/src/stdio/CMakeLists.txt b/libc/src/stdio/CMakeLists.txt
index 1056f38fc7513..deb7e5ad942bf 100644
--- a/libc/src/stdio/CMakeLists.txt
+++ b/libc/src/stdio/CMakeLists.txt
@@ -263,6 +263,13 @@ add_entrypoint_object(
     .${LIBC_TARGET_OS}.rename
 )
 
+add_entrypoint_object(
+  fdopen
+  ALIAS
+  DEPENDS
+    .${LIBC_TARGET_OS}.fdopen
+)
+
 # These entrypoints have multiple potential implementations.
 add_stdio_entrypoint_object(feof)
 add_stdio_entrypoint_object(feof_unlocked)
diff --git a/libc/src/stdio/fdopen.h b/libc/src/stdio/fdopen.h
new file mode 100644
index 0000000000000..158a133e7e131
--- /dev/null
+++ b/libc/src/stdio/fdopen.h
@@ -0,0 +1,20 @@
+//===-- Implementation header of open ---------------------------*- C++ -*-===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIBC_SRC_STDIO_FDOPEN_H
+#define LLVM_LIBC_SRC_STDIO_FDOPEN_H
+
+#include <stdio.h>
+
+namespace LIBC_NAMESPACE {
+
+FILE *fdopen(int fd, const char *mode);
+
+} // namespace LIBC_NAMESPACE
+
+#endif // LLVM_LIBC_SRC_STDIO_FDOPEN_H
diff --git a/libc/src/stdio/linux/CMakeLists.txt b/libc/src/stdio/linux/CMakeLists.txt
index a08ff0ba4832f..fe8b529d094fb 100644
--- a/libc/src/stdio/linux/CMakeLists.txt
+++ b/libc/src/stdio/linux/CMakeLists.txt
@@ -24,3 +24,18 @@ add_entrypoint_object(
     libc.src.__support.OSUtil.osutil
     libc.src.errno.errno
 )
+
+add_entrypoint_object(
+  fdopen
+  SRCS
+    fdopen.cpp
+  HDRS
+    ../fdopen.h
+  DEPENDS
+    libc.include.fcntl
+    libc.include.stdio
+    libc.src.errno.errno
+    libc.src.fcntl.fcntl
+    libc.src.stdio.fseek
+    libc.src.__support.File.linux.file
+)
\ No newline at end of file
diff --git a/libc/src/stdio/linux/fdopen.cpp b/libc/src/stdio/linux/fdopen.cpp
new file mode 100644
index 0000000000000..2f4c69f58b92d
--- /dev/null
+++ b/libc/src/stdio/linux/fdopen.cpp
@@ -0,0 +1,79 @@
+//===-- Implementation of fprintf -------------------------------*- C++ -*-===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#include "src/stdio/fdopen.h"
+
+#include "include/llvm-libc-macros/generic-error-number-macros.h"
+#include "include/llvm-libc-macros/linux/fcntl-macros.h"
+#include "src/__support/File/linux/file.h"
+#include "src/errno/libc_errno.h"
+#include "src/fcntl/fcntl.h"
+#include "src/stdio/fseek.h"
+
+#include <stdio.h>
+
+namespace LIBC_NAMESPACE {
+
+LLVM_LIBC_FUNCTION(::FILE *, fdopen, (int fd, const char *mode)) {
+  using ModeFlags = File::ModeFlags;
+  ModeFlags modeflags = File::mode_flags(mode);
+  if (modeflags == 0) {
+    libc_errno = EINVAL;
+    return nullptr;
+  }
+
+  int fd_flags = LIBC_NAMESPACE::fcntl(fd, F_GETFL);
+  if (fd_flags == -1) {
+    return nullptr;
+  }
+
+  using OpenMode = File::OpenMode;
+  if (((fd_flags & O_ACCMODE) == O_RDONLY &&
+       !(modeflags & static_cast<ModeFlags>(OpenMode::READ))) ||
+      ((fd_flags & O_ACCMODE) == O_WRONLY &&
+       !(modeflags & static_cast<ModeFlags>(OpenMode::WRITE)))) {
+    libc_errno = EINVAL;
+    return nullptr;
+  }
+
+  bool do_seek = false;
+  if ((modeflags & static_cast<ModeFlags>(OpenMode::APPEND)) &&
+      !(fd_flags & O_APPEND)) {
+    do_seek = true;
+    if (LIBC_NAMESPACE::fcntl(fd, F_SETFL, fd_flags | O_APPEND) == -1) {
+      libc_errno = EBADF;
+      return nullptr;
+    }
+  }
+
+  uint8_t *buffer;
+  {
+    AllocChecker ac;
+    buffer = new (ac) uint8_t[File::DEFAULT_BUFFER_SIZE];
+    if (!ac) {
+      libc_errno = ENOMEM;
+      return nullptr;
+    }
+  }
+  AllocChecker ac;
+  auto *linux_file = new (ac)
+      LinuxFile(fd, buffer, File::DEFAULT_BUFFER_SIZE, _IOFBF, true, modeflags);
+  if (!ac) {
+    libc_errno = ENOMEM;
+    return nullptr;
+  }
+  auto *file = reinterpret_cast<::FILE *>(linux_file);
+  if (do_seek && LIBC_NAMESPACE::fseek(file, 0, SEEK_END) != 0) {
+    free(linux_file);
+    return nullptr;
+  }
+
+  return file;
+}
+
+} // namespace LIBC_NAMESPACE
diff --git a/libc/test/src/fcntl/fcntl_test.cpp b/libc/test/src/fcntl/fcntl_test.cpp
index c5cbb61b4ed8a..e0641b2e3fc6b 100644
--- a/libc/test/src/fcntl/fcntl_test.cpp
+++ b/libc/test/src/fcntl/fcntl_test.cpp
@@ -153,3 +153,13 @@ TEST(LlvmLibcFcntlTest, FcntlGetLkWrite) {
 
   ASSERT_THAT(LIBC_NAMESPACE::close(fd), Succeeds(0));
 }
+
+TEST(LlvmLibcFcntlTest, UseAfterClose) {
+  using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Succeeds;
+  constexpr const char *TEST_FILE_NAME = "testdata/fcntl_use_after_close.test";
+  auto TEST_FILE = libc_make_test_file_path(TEST_FILE_NAME);
+  int fd = LIBC_NAMESPACE::open(TEST_FILE, O_CREAT | O_TRUNC | O_RDWR, S_IRWXU);
+  ASSERT_THAT(LIBC_NAMESPACE::close(fd), Succeeds(0));
+  ASSERT_EQ(-1, LIBC_NAMESPACE::fcntl(fd, F_GETFL));
+  ASSERT_ERRNO_EQ(EBADF);
+}
\ No newline at end of file
diff --git a/libc/test/src/stdio/CMakeLists.txt b/libc/test/src/stdio/CMakeLists.txt
index 03c43eaefebe5..45f3147089d06 100644
--- a/libc/test/src/stdio/CMakeLists.txt
+++ b/libc/test/src/stdio/CMakeLists.txt
@@ -369,6 +369,20 @@ if(${LIBC_TARGET_OS} STREQUAL "linux")
       libc.src.unistd.close
       libc.test.UnitTest.ErrnoSetterMatcher
   )
+
+  add_libc_test(
+    fdopen_test
+    SUITE
+      libc_stdio_unittests
+    SRCS
+      fdopen_test.cpp
+    DEPENDS
+      libc.src.fcntl.open
+      libc.src.stdio.fclose
+      libc.src.stdio.fdopen
+      libc.src.unistd.close
+      libc.test.UnitTest.ErrnoSetterMatcher
+  )
 endif()
 
 add_libc_test(
diff --git a/libc/test/src/stdio/fdopen_test.cpp b/libc/test/src/stdio/fdopen_test.cpp
new file mode 100644
index 0000000000000..399321b2bf8c8
--- /dev/null
+++ b/libc/test/src/stdio/fdopen_test.cpp
@@ -0,0 +1,68 @@
+//===-- Unittest for fcntl ------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#include "include/llvm-libc-macros/linux/fcntl-macros.h"
+#include "src/stdio/fdopen.h"
+
+#include "src/errno/libc_errno.h"
+#include "src/fcntl/fcntl.h"
+#include "src/fcntl/open.h"
+#include "src/stdio/fclose.h"
+#include "src/unistd/close.h"
+#include "test/UnitTest/ErrnoSetterMatcher.h"
+#include "test/UnitTest/LibcTest.h"
+#include "test/UnitTest/Test.h"
+
+#include <sys/stat.h> // For S_IRWXU
+
+TEST(LlvmLibcStdioFdopenTest, InvalidInput) {
+  using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Succeeds;
+  LIBC_NAMESPACE::libc_errno = 0;
+  constexpr const char *TEST_FILE_NAME = "testdata/fdopen_invalid_inputs.test";
+  auto TEST_FILE = libc_make_test_file_path(TEST_FILE_NAME);
+  int fd = LIBC_NAMESPACE::open(TEST_FILE, O_CREAT | O_TRUNC | O_RDONLY);
+  auto *fp = LIBC_NAMESPACE::fdopen(fd, "m+");
+  ASSERT_ERRNO_EQ(EINVAL);
+  ASSERT_TRUE(nullptr == fp);
+  LIBC_NAMESPACE::close(fd);
+  LIBC_NAMESPACE::libc_errno = 0;
+  fp = LIBC_NAMESPACE::fdopen(fd, "r");
+  ASSERT_ERRNO_EQ(EBADF);
+  ASSERT_TRUE(nullptr == fp);
+}
+
+TEST(LlvmLibcStdioFdopenTest, InvalidMode) {
+  using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Succeeds;
+  LIBC_NAMESPACE::libc_errno = 0;
+  constexpr const char *TEST_FILE_NAME = "testdata/fdopen_invid_mode.test";
+  auto TEST_FILE = libc_make_test_file_path(TEST_FILE_NAME);
+  int fd = LIBC_NAMESPACE::open(TEST_FILE, O_CREAT | O_TRUNC | O_RDONLY);
+  ASSERT_ERRNO_SUCCESS();
+  ASSERT_GT(fd, 0);
+  auto *fp = LIBC_NAMESPACE::fdopen(fd, "r+");
+  ASSERT_ERRNO_SUCCESS();
+  ASSERT_TRUE(nullptr != fp);
+  ASSERT_THAT(LIBC_NAMESPACE::fclose(fp), Succeeds(0));
+  // If the mode argument is invalid, then `fdopen` returns a nullptr and sets
+  // the `errno` to EINVAL. In this case the `mode` param can only be "r" or
+  // "r+"
+  int fd2 = LIBC_NAMESPACE::open(TEST_FILE, O_CREAT | O_TRUNC | O_RDONLY);
+  ASSERT_ERRNO_SUCCESS();
+  ASSERT_GT(fd2, 0);
+  auto *fp2 = LIBC_NAMESPACE::fdopen(fd2, "w");
+  ASSERT_ERRNO_EQ(EINVAL);
+  ASSERT_TRUE(nullptr == fp2);
+}
+
+TEST(LlvmLibcStdioFdopenTest, WriteRead) {
+
+}
+
+TEST(LlvmLibcStdioFdopenTest, Append) {}
+
+TEST(LlvmLibcStdioFdopenTest, AppendPlus) {}
\ No newline at end of file

>From 7eee9b3652f420492ded381a810316fc7bd94b2d Mon Sep 17 00:00:00 2001
From: Xu Zhang <simonzgx at gmail.com>
Date: Tue, 4 Jun 2024 23:52:28 +0800
Subject: [PATCH 2/9] Update unit test

---
 libc/src/stdio/linux/fdopen.cpp     |  1 -
 libc/test/src/stdio/CMakeLists.txt  |  2 +
 libc/test/src/stdio/fdopen_test.cpp | 77 ++++++++++++++++++-----------
 3 files changed, 51 insertions(+), 29 deletions(-)

diff --git a/libc/src/stdio/linux/fdopen.cpp b/libc/src/stdio/linux/fdopen.cpp
index 2f4c69f58b92d..6d35e68853917 100644
--- a/libc/src/stdio/linux/fdopen.cpp
+++ b/libc/src/stdio/linux/fdopen.cpp
@@ -46,7 +46,6 @@ LLVM_LIBC_FUNCTION(::FILE *, fdopen, (int fd, const char *mode)) {
       !(fd_flags & O_APPEND)) {
     do_seek = true;
     if (LIBC_NAMESPACE::fcntl(fd, F_SETFL, fd_flags | O_APPEND) == -1) {
-      libc_errno = EBADF;
       return nullptr;
     }
   }
diff --git a/libc/test/src/stdio/CMakeLists.txt b/libc/test/src/stdio/CMakeLists.txt
index 45f3147089d06..5eb8c9577893b 100644
--- a/libc/test/src/stdio/CMakeLists.txt
+++ b/libc/test/src/stdio/CMakeLists.txt
@@ -380,6 +380,8 @@ if(${LIBC_TARGET_OS} STREQUAL "linux")
       libc.src.fcntl.open
       libc.src.stdio.fclose
       libc.src.stdio.fdopen
+      libc.src.stdio.fgets
+      libc.src.stdio.fputs
       libc.src.unistd.close
       libc.test.UnitTest.ErrnoSetterMatcher
   )
diff --git a/libc/test/src/stdio/fdopen_test.cpp b/libc/test/src/stdio/fdopen_test.cpp
index 399321b2bf8c8..4cf693a0d9d01 100644
--- a/libc/test/src/stdio/fdopen_test.cpp
+++ b/libc/test/src/stdio/fdopen_test.cpp
@@ -10,9 +10,10 @@
 #include "src/stdio/fdopen.h"
 
 #include "src/errno/libc_errno.h"
-#include "src/fcntl/fcntl.h"
 #include "src/fcntl/open.h"
 #include "src/stdio/fclose.h"
+#include "src/stdio/fgets.h"
+#include "src/stdio/fputs.h"
 #include "src/unistd/close.h"
 #include "test/UnitTest/ErrnoSetterMatcher.h"
 #include "test/UnitTest/LibcTest.h"
@@ -20,49 +21,69 @@
 
 #include <sys/stat.h> // For S_IRWXU
 
-TEST(LlvmLibcStdioFdopenTest, InvalidInput) {
+TEST(LlvmLibcStdioFdopenTest, WriteAppendRead) {
   using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Succeeds;
   LIBC_NAMESPACE::libc_errno = 0;
-  constexpr const char *TEST_FILE_NAME = "testdata/fdopen_invalid_inputs.test";
+  constexpr const char *TEST_FILE_NAME = "testdata/write_read_append.test";
   auto TEST_FILE = libc_make_test_file_path(TEST_FILE_NAME);
-  int fd = LIBC_NAMESPACE::open(TEST_FILE, O_CREAT | O_TRUNC | O_RDONLY);
-  auto *fp = LIBC_NAMESPACE::fdopen(fd, "m+");
-  ASSERT_ERRNO_EQ(EINVAL);
-  ASSERT_TRUE(nullptr == fp);
-  LIBC_NAMESPACE::close(fd);
+  int fd = LIBC_NAMESPACE::open(TEST_FILE, O_CREAT | O_TRUNC | O_RDWR, S_IRWXU);
+  auto *fp = LIBC_NAMESPACE::fdopen(fd, "w");
+  ASSERT_ERRNO_SUCCESS();
+  ASSERT_TRUE(nullptr != fp);
+  constexpr const char HELLO[] = "Hello";
+  LIBC_NAMESPACE::fputs(HELLO, fp);
+  LIBC_NAMESPACE::fclose(fp);
+  ASSERT_ERRNO_SUCCESS();
+
+  constexpr const char LLVM[] = "LLVM";
+  int fd2 = LIBC_NAMESPACE::open(TEST_FILE, O_CREAT | O_RDWR);
+  auto *fp2 = LIBC_NAMESPACE::fdopen(fd2, "a");
+  LIBC_NAMESPACE::fputs(LLVM, fp2);
+  LIBC_NAMESPACE::fclose(fp2);
+  ASSERT_ERRNO_SUCCESS();
+
+  int fd3 = LIBC_NAMESPACE::open(TEST_FILE, O_CREAT | O_RDWR);
+  auto *fp3 = LIBC_NAMESPACE::fdopen(fd3, "r");
+  char buffer[10];
+  LIBC_NAMESPACE::fgets(buffer, sizeof(buffer), fp3);
+  EXPECT_EQ('H', buffer[0]);
+  EXPECT_EQ('M', buffer[8]);
+  LIBC_NAMESPACE::fclose(fp3);
+  ASSERT_ERRNO_SUCCESS();
+}
+
+TEST(LlvmLibcStdioFdopenTest, InvalidFd) {
   LIBC_NAMESPACE::libc_errno = 0;
-  fp = LIBC_NAMESPACE::fdopen(fd, "r");
+  constexpr const char *TEST_FILE_NAME = "testdata/invalid_fd.test";
+  auto TEST_FILE = libc_make_test_file_path(TEST_FILE_NAME);
+  int fd = LIBC_NAMESPACE::open(TEST_FILE, O_CREAT | O_TRUNC);
+  LIBC_NAMESPACE::close(fd);
+  // With `fd` already closed, `fdopen` should fail and set the `errno` to EBADF
+  auto *fp = LIBC_NAMESPACE::fdopen(fd, "r");
   ASSERT_ERRNO_EQ(EBADF);
   ASSERT_TRUE(nullptr == fp);
 }
 
 TEST(LlvmLibcStdioFdopenTest, InvalidMode) {
-  using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Succeeds;
   LIBC_NAMESPACE::libc_errno = 0;
-  constexpr const char *TEST_FILE_NAME = "testdata/fdopen_invid_mode.test";
+  constexpr const char *TEST_FILE_NAME = "testdata/invalid_mode.test";
   auto TEST_FILE = libc_make_test_file_path(TEST_FILE_NAME);
-  int fd = LIBC_NAMESPACE::open(TEST_FILE, O_CREAT | O_TRUNC | O_RDONLY);
+  int fd = LIBC_NAMESPACE::open(TEST_FILE, O_CREAT | O_RDONLY, S_IRWXU);
   ASSERT_ERRNO_SUCCESS();
   ASSERT_GT(fd, 0);
-  auto *fp = LIBC_NAMESPACE::fdopen(fd, "r+");
-  ASSERT_ERRNO_SUCCESS();
-  ASSERT_TRUE(nullptr != fp);
-  ASSERT_THAT(LIBC_NAMESPACE::fclose(fp), Succeeds(0));
+
+  // `Mode` must be one of "r", "w" or "a"
+  auto *fp = LIBC_NAMESPACE::fdopen(fd, "m+");
+  ASSERT_ERRNO_EQ(EINVAL);
+  ASSERT_TRUE(nullptr == fp);
+
   // If the mode argument is invalid, then `fdopen` returns a nullptr and sets
   // the `errno` to EINVAL. In this case the `mode` param can only be "r" or
   // "r+"
-  int fd2 = LIBC_NAMESPACE::open(TEST_FILE, O_CREAT | O_TRUNC | O_RDONLY);
-  ASSERT_ERRNO_SUCCESS();
-  ASSERT_GT(fd2, 0);
-  auto *fp2 = LIBC_NAMESPACE::fdopen(fd2, "w");
+  auto *fp2 = LIBC_NAMESPACE::fdopen(fd, "w");
   ASSERT_ERRNO_EQ(EINVAL);
   ASSERT_TRUE(nullptr == fp2);
+  LIBC_NAMESPACE::libc_errno = 0;
+  LIBC_NAMESPACE::close(fd);
+  ASSERT_ERRNO_SUCCESS();
 }
-
-TEST(LlvmLibcStdioFdopenTest, WriteRead) {
-
-}
-
-TEST(LlvmLibcStdioFdopenTest, Append) {}
-
-TEST(LlvmLibcStdioFdopenTest, AppendPlus) {}
\ No newline at end of file

>From 0251116029b0e728e42463076dfcf41e7ff18aa0 Mon Sep 17 00:00:00 2001
From: Xu Zhang <simonzgx at gmail.com>
Date: Tue, 4 Jun 2024 23:59:06 +0800
Subject: [PATCH 3/9] Add a blank line

---
 libc/src/stdio/linux/CMakeLists.txt | 2 +-
 libc/test/src/fcntl/fcntl_test.cpp  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/libc/src/stdio/linux/CMakeLists.txt b/libc/src/stdio/linux/CMakeLists.txt
index fe8b529d094fb..5b6baf5822dcc 100644
--- a/libc/src/stdio/linux/CMakeLists.txt
+++ b/libc/src/stdio/linux/CMakeLists.txt
@@ -38,4 +38,4 @@ add_entrypoint_object(
     libc.src.fcntl.fcntl
     libc.src.stdio.fseek
     libc.src.__support.File.linux.file
-)
\ No newline at end of file
+)
diff --git a/libc/test/src/fcntl/fcntl_test.cpp b/libc/test/src/fcntl/fcntl_test.cpp
index e0641b2e3fc6b..fc909acc1aa0b 100644
--- a/libc/test/src/fcntl/fcntl_test.cpp
+++ b/libc/test/src/fcntl/fcntl_test.cpp
@@ -162,4 +162,4 @@ TEST(LlvmLibcFcntlTest, UseAfterClose) {
   ASSERT_THAT(LIBC_NAMESPACE::close(fd), Succeeds(0));
   ASSERT_EQ(-1, LIBC_NAMESPACE::fcntl(fd, F_GETFL));
   ASSERT_ERRNO_EQ(EBADF);
-}
\ No newline at end of file
+}

>From a926d0fad7ac304def50056bef93268354d93cc2 Mon Sep 17 00:00:00 2001
From: Xu Zhang <simonzgx at gmail.com>
Date: Wed, 5 Jun 2024 13:57:49 +0800
Subject: [PATCH 4/9] Update cmake dependency

---
 libc/src/stdio/linux/CMakeLists.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libc/src/stdio/linux/CMakeLists.txt b/libc/src/stdio/linux/CMakeLists.txt
index 5b6baf5822dcc..1a6ead5dba931 100644
--- a/libc/src/stdio/linux/CMakeLists.txt
+++ b/libc/src/stdio/linux/CMakeLists.txt
@@ -37,5 +37,5 @@ add_entrypoint_object(
     libc.src.errno.errno
     libc.src.fcntl.fcntl
     libc.src.stdio.fseek
-    libc.src.__support.File.linux.file
+    libc.src.__support.File.platform_file
 )

>From 9dd142938363aa6efffff2bf782d547210b4fa49 Mon Sep 17 00:00:00 2001
From: Xu Zhang <simonzgx at gmail.com>
Date: Wed, 5 Jun 2024 14:43:16 +0800
Subject: [PATCH 5/9] Remove unused header files

---
 libc/src/stdio/linux/fdopen.cpp     | 2 --
 libc/test/src/stdio/fdopen_test.cpp | 1 -
 2 files changed, 3 deletions(-)

diff --git a/libc/src/stdio/linux/fdopen.cpp b/libc/src/stdio/linux/fdopen.cpp
index 6d35e68853917..d9529dbc05848 100644
--- a/libc/src/stdio/linux/fdopen.cpp
+++ b/libc/src/stdio/linux/fdopen.cpp
@@ -15,8 +15,6 @@
 #include "src/fcntl/fcntl.h"
 #include "src/stdio/fseek.h"
 
-#include <stdio.h>
-
 namespace LIBC_NAMESPACE {
 
 LLVM_LIBC_FUNCTION(::FILE *, fdopen, (int fd, const char *mode)) {
diff --git a/libc/test/src/stdio/fdopen_test.cpp b/libc/test/src/stdio/fdopen_test.cpp
index 4cf693a0d9d01..15020ae747b78 100644
--- a/libc/test/src/stdio/fdopen_test.cpp
+++ b/libc/test/src/stdio/fdopen_test.cpp
@@ -16,7 +16,6 @@
 #include "src/stdio/fputs.h"
 #include "src/unistd/close.h"
 #include "test/UnitTest/ErrnoSetterMatcher.h"
-#include "test/UnitTest/LibcTest.h"
 #include "test/UnitTest/Test.h"
 
 #include <sys/stat.h> // For S_IRWXU

>From a535a9dcf8153c5f96da2cf30d3b2f6a049876f4 Mon Sep 17 00:00:00 2001
From: Xu Zhang <simonzgx at gmail.com>
Date: Sat, 8 Jun 2024 00:28:10 +0800
Subject: [PATCH 6/9] Move implementation of fcntl to OSUtil dir & resolve
 comments

---
 libc/src/__support/OSUtil/fcntl.h             | 19 ++++
 .../src/__support/OSUtil/linux/CMakeLists.txt |  6 ++
 libc/src/__support/OSUtil/linux/fcntl.cpp     | 94 +++++++++++++++++++
 libc/src/fcntl/linux/CMakeLists.txt           |  5 -
 libc/src/fcntl/linux/fcntl.cpp                | 79 +---------------
 libc/src/stdio/linux/CMakeLists.txt           |  3 +-
 libc/src/stdio/linux/fdopen.cpp               | 29 +++---
 libc/test/src/stdio/fdopen_test.cpp           |  6 +-
 8 files changed, 144 insertions(+), 97 deletions(-)
 create mode 100644 libc/src/__support/OSUtil/fcntl.h
 create mode 100644 libc/src/__support/OSUtil/linux/fcntl.cpp

diff --git a/libc/src/__support/OSUtil/fcntl.h b/libc/src/__support/OSUtil/fcntl.h
new file mode 100644
index 0000000000000..9121576d57c86
--- /dev/null
+++ b/libc/src/__support/OSUtil/fcntl.h
@@ -0,0 +1,19 @@
+//===-- Implementation header of internal fcntl function ---------------*- C++
+//-*-===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIBC_SRC___SUPPORT_OSUTIL_FCNTL_H
+#define LLVM_LIBC_SRC___SUPPORT_OSUTIL_FCNTL_H
+
+namespace LIBC_NAMESPACE::internal {
+
+int fcntl(int fd, int cmd, void *arg = nullptr);
+
+} // namespace LIBC_NAMESPACE::internal
+
+#endif // LLVM_LIBC_SRC___SUPPORT_OSUTIL_FCNTL_H
diff --git a/libc/src/__support/OSUtil/linux/CMakeLists.txt b/libc/src/__support/OSUtil/linux/CMakeLists.txt
index 9a55232d532fe..78b117fd19439 100644
--- a/libc/src/__support/OSUtil/linux/CMakeLists.txt
+++ b/libc/src/__support/OSUtil/linux/CMakeLists.txt
@@ -8,6 +8,7 @@ add_object_library(
   linux_util
   SRCS
     exit.cpp
+    fcntl.cpp
   HDRS
     io.h
     syscall.h
@@ -15,4 +16,9 @@ add_object_library(
     .${LIBC_TARGET_ARCHITECTURE}.linux_${LIBC_TARGET_ARCHITECTURE}_util
     libc.src.__support.common
     libc.src.__support.CPP.string_view
+    libc.src.errno.errno
+    libc.hdr.fcntl_macros
+    libc.hdr.types.struct_flock
+    libc.hdr.types.struct_flock64
+    libc.hdr.types.struct_f_owner_ex
 )
diff --git a/libc/src/__support/OSUtil/linux/fcntl.cpp b/libc/src/__support/OSUtil/linux/fcntl.cpp
new file mode 100644
index 0000000000000..7dc416a7916df
--- /dev/null
+++ b/libc/src/__support/OSUtil/linux/fcntl.cpp
@@ -0,0 +1,94 @@
+//===-- Implementation of internal fcntl ----------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#include "src/__support/OSUtil/fcntl.h"
+
+#include "hdr/fcntl_macros.h"
+#include "hdr/types/struct_f_owner_ex.h"
+#include "hdr/types/struct_flock.h"
+#include "hdr/types/struct_flock64.h"
+#include "src/__support/OSUtil/syscall.h" // For internal syscall function.
+#include "src/__support/common.h"
+#include "src/errno/libc_errno.h"
+
+#include <stdarg.h>
+#include <sys/syscall.h> // For syscall numbers.
+
+namespace LIBC_NAMESPACE::internal {
+
+int fcntl(int fd, int cmd, void *arg) {
+  switch (cmd) {
+  case F_OFD_SETLKW: {
+    struct flock *flk = reinterpret_cast<struct flock *>(arg);
+    // convert the struct to a flock64
+    struct flock64 flk64;
+    flk64.l_type = flk->l_type;
+    flk64.l_whence = flk->l_whence;
+    flk64.l_start = flk->l_start;
+    flk64.l_len = flk->l_len;
+    flk64.l_pid = flk->l_pid;
+    // create a syscall
+    return LIBC_NAMESPACE::syscall_impl<int>(SYS_fcntl, fd, cmd, &flk64);
+  }
+  case F_OFD_GETLK:
+  case F_OFD_SETLK: {
+    struct flock *flk = reinterpret_cast<struct flock *>(arg);
+    // convert the struct to a flock64
+    struct flock64 flk64;
+    flk64.l_type = flk->l_type;
+    flk64.l_whence = flk->l_whence;
+    flk64.l_start = flk->l_start;
+    flk64.l_len = flk->l_len;
+    flk64.l_pid = flk->l_pid;
+    // create a syscall
+    int retVal = LIBC_NAMESPACE::syscall_impl<int>(SYS_fcntl, fd, cmd, &flk64);
+    // On failure, return
+    if (retVal == -1)
+      return -1;
+    // Check for overflow, i.e. the offsets are not the same when cast
+    // to off_t from off64_t.
+    if (static_cast<off_t>(flk64.l_len) != flk64.l_len ||
+        static_cast<off_t>(flk64.l_start) != flk64.l_start) {
+      libc_errno = EOVERFLOW;
+      return -1;
+    }
+    // Now copy back into flk, in case flk64 got modified
+    flk->l_type = flk64.l_type;
+    flk->l_whence = flk64.l_whence;
+    flk->l_start = flk64.l_start;
+    flk->l_len = flk64.l_len;
+    flk->l_pid = flk64.l_pid;
+    return retVal;
+  }
+  case F_GETOWN: {
+    struct f_owner_ex fex;
+    int retVal =
+        LIBC_NAMESPACE::syscall_impl<int>(SYS_fcntl, fd, F_GETOWN_EX, &fex);
+    if (retVal == -EINVAL)
+      return LIBC_NAMESPACE::syscall_impl<int>(SYS_fcntl, fd, cmd,
+                                               reinterpret_cast<void *>(arg));
+    if (static_cast<unsigned long>(retVal) <= -4096UL)
+      return fex.type == F_OWNER_PGRP ? -fex.pid : fex.pid;
+
+    libc_errno = -retVal;
+    return -1;
+  }
+  // The general case
+  default: {
+    int retVal = LIBC_NAMESPACE::syscall_impl<int>(
+        SYS_fcntl, fd, cmd, reinterpret_cast<void *>(arg));
+    if (retVal >= 0) {
+      return retVal;
+    }
+    libc_errno = -retVal;
+    return -1;
+  }
+  }
+}
+
+} // namespace LIBC_NAMESPACE::internal
diff --git a/libc/src/fcntl/linux/CMakeLists.txt b/libc/src/fcntl/linux/CMakeLists.txt
index 732b7beac41bf..ee8ae63b8cf06 100644
--- a/libc/src/fcntl/linux/CMakeLists.txt
+++ b/libc/src/fcntl/linux/CMakeLists.txt
@@ -18,12 +18,7 @@ add_entrypoint_object(
     ../fcntl.h
   DEPENDS
     libc.include.fcntl
-    libc.hdr.types.struct_flock
-    libc.hdr.types.struct_flock64
-    libc.hdr.types.struct_f_owner_ex
-    libc.hdr.fcntl_macros
     libc.src.__support.OSUtil.osutil
-    libc.src.errno.errno
 )
 
 add_entrypoint_object(
diff --git a/libc/src/fcntl/linux/fcntl.cpp b/libc/src/fcntl/linux/fcntl.cpp
index 4100ec4298a20..3875889d9d0b9 100644
--- a/libc/src/fcntl/linux/fcntl.cpp
+++ b/libc/src/fcntl/linux/fcntl.cpp
@@ -8,91 +8,20 @@
 
 #include "src/fcntl/fcntl.h"
 
-#include "hdr/fcntl_macros.h"
-#include "hdr/types/struct_f_owner_ex.h"
-#include "hdr/types/struct_flock.h"
-#include "hdr/types/struct_flock64.h"
-#include "src/__support/OSUtil/syscall.h" // For internal syscall function.
+#include "src/__support/OSUtil/fcntl.h"
 #include "src/__support/common.h"
-#include "src/errno/libc_errno.h"
 
 #include <stdarg.h>
-#include <sys/syscall.h> // For syscall numbers.
 
-// The OFD file locks require special handling for LARGEFILES
 namespace LIBC_NAMESPACE {
+
 LLVM_LIBC_FUNCTION(int, fcntl, (int fd, int cmd, ...)) {
   void *arg;
   va_list varargs;
   va_start(varargs, cmd);
   arg = va_arg(varargs, void *);
   va_end(varargs);
-
-  switch (cmd) {
-  case F_OFD_SETLKW: {
-    struct flock *flk = reinterpret_cast<struct flock *>(arg);
-    // convert the struct to a flock64
-    struct flock64 flk64;
-    flk64.l_type = flk->l_type;
-    flk64.l_whence = flk->l_whence;
-    flk64.l_start = flk->l_start;
-    flk64.l_len = flk->l_len;
-    flk64.l_pid = flk->l_pid;
-    // create a syscall
-    return syscall_impl<int>(SYS_fcntl, fd, cmd, &flk64);
-  }
-  case F_OFD_GETLK:
-  case F_OFD_SETLK: {
-    struct flock *flk = reinterpret_cast<struct flock *>(arg);
-    // convert the struct to a flock64
-    struct flock64 flk64;
-    flk64.l_type = flk->l_type;
-    flk64.l_whence = flk->l_whence;
-    flk64.l_start = flk->l_start;
-    flk64.l_len = flk->l_len;
-    flk64.l_pid = flk->l_pid;
-    // create a syscall
-    int retVal = syscall_impl<int>(SYS_fcntl, fd, cmd, &flk64);
-    // On failure, return
-    if (retVal == -1)
-      return -1;
-    // Check for overflow, i.e. the offsets are not the same when cast
-    // to off_t from off64_t.
-    if (static_cast<off_t>(flk64.l_len) != flk64.l_len ||
-        static_cast<off_t>(flk64.l_start) != flk64.l_start) {
-      libc_errno = EOVERFLOW;
-      return -1;
-    }
-    // Now copy back into flk, in case flk64 got modified
-    flk->l_type = flk64.l_type;
-    flk->l_whence = flk64.l_whence;
-    flk->l_start = flk64.l_start;
-    flk->l_len = flk64.l_len;
-    flk->l_pid = flk64.l_pid;
-    return retVal;
-  }
-  case F_GETOWN: {
-    struct f_owner_ex fex;
-    int retVal = syscall_impl<int>(SYS_fcntl, fd, F_GETOWN_EX, &fex);
-    if (retVal == -EINVAL)
-      return syscall_impl<int>(SYS_fcntl, fd, cmd,
-                               reinterpret_cast<void *>(arg));
-    if (static_cast<unsigned long>(retVal) <= -4096UL)
-      return fex.type == F_OWNER_PGRP ? -fex.pid : fex.pid;
-
-    libc_errno = -retVal;
-    return -1;
-  }
-  // The general case
-  default: {
-    int retVal =
-        syscall_impl<int>(SYS_fcntl, fd, cmd, reinterpret_cast<void *>(arg));
-    if (retVal >= 0) {
-      return retVal;
-    }
-    libc_errno = -retVal;
-    return -1;
-  }
-  }
+  return LIBC_NAMESPACE::internal::fcntl(fd, cmd, arg);
 }
+
 } // namespace LIBC_NAMESPACE
diff --git a/libc/src/stdio/linux/CMakeLists.txt b/libc/src/stdio/linux/CMakeLists.txt
index 1a6ead5dba931..80998f0d4bfa9 100644
--- a/libc/src/stdio/linux/CMakeLists.txt
+++ b/libc/src/stdio/linux/CMakeLists.txt
@@ -32,10 +32,9 @@ add_entrypoint_object(
   HDRS
     ../fdopen.h
   DEPENDS
-    libc.include.fcntl
     libc.include.stdio
     libc.src.errno.errno
-    libc.src.fcntl.fcntl
     libc.src.stdio.fseek
     libc.src.__support.File.platform_file
+    libc.src.__support.OSUtil.osutil
 )
diff --git a/libc/src/stdio/linux/fdopen.cpp b/libc/src/stdio/linux/fdopen.cpp
index d9529dbc05848..48fc3f880becb 100644
--- a/libc/src/stdio/linux/fdopen.cpp
+++ b/libc/src/stdio/linux/fdopen.cpp
@@ -1,4 +1,4 @@
-//===-- Implementation of fprintf -------------------------------*- C++ -*-===//
+//===-- Implementation of fdopen --------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -8,12 +8,11 @@
 
 #include "src/stdio/fdopen.h"
 
-#include "include/llvm-libc-macros/generic-error-number-macros.h"
-#include "include/llvm-libc-macros/linux/fcntl-macros.h"
+#include "hdr/errno_macros.h"
+#include "include/llvm-libc-macros/fcntl-macros.h"
 #include "src/__support/File/linux/file.h"
+#include "src/__support/OSUtil/fcntl.h"
 #include "src/errno/libc_errno.h"
-#include "src/fcntl/fcntl.h"
-#include "src/stdio/fseek.h"
 
 namespace LIBC_NAMESPACE {
 
@@ -25,7 +24,7 @@ LLVM_LIBC_FUNCTION(::FILE *, fdopen, (int fd, const char *mode)) {
     return nullptr;
   }
 
-  int fd_flags = LIBC_NAMESPACE::fcntl(fd, F_GETFL);
+  int fd_flags = internal::fcntl(fd, F_GETFL);
   if (fd_flags == -1) {
     return nullptr;
   }
@@ -43,7 +42,8 @@ LLVM_LIBC_FUNCTION(::FILE *, fdopen, (int fd, const char *mode)) {
   if ((modeflags & static_cast<ModeFlags>(OpenMode::APPEND)) &&
       !(fd_flags & O_APPEND)) {
     do_seek = true;
-    if (LIBC_NAMESPACE::fcntl(fd, F_SETFL, fd_flags | O_APPEND) == -1) {
+    if (internal::fcntl(fd, F_SETFL,
+                        reinterpret_cast<void *>(fd_flags | O_APPEND)) == -1) {
       return nullptr;
     }
   }
@@ -58,19 +58,22 @@ LLVM_LIBC_FUNCTION(::FILE *, fdopen, (int fd, const char *mode)) {
     }
   }
   AllocChecker ac;
-  auto *linux_file = new (ac)
+  auto *file = new (ac)
       LinuxFile(fd, buffer, File::DEFAULT_BUFFER_SIZE, _IOFBF, true, modeflags);
   if (!ac) {
     libc_errno = ENOMEM;
     return nullptr;
   }
-  auto *file = reinterpret_cast<::FILE *>(linux_file);
-  if (do_seek && LIBC_NAMESPACE::fseek(file, 0, SEEK_END) != 0) {
-    free(linux_file);
-    return nullptr;
+  if (do_seek) {
+    auto result = file->seek(0, SEEK_END);
+    if (!result.has_value()) {
+      libc_errno = result.error();
+      free(file);
+      return nullptr;
+    }
   }
 
-  return file;
+  return reinterpret_cast<::FILE *>(file);
 }
 
 } // namespace LIBC_NAMESPACE
diff --git a/libc/test/src/stdio/fdopen_test.cpp b/libc/test/src/stdio/fdopen_test.cpp
index 15020ae747b78..80e249e9d8dce 100644
--- a/libc/test/src/stdio/fdopen_test.cpp
+++ b/libc/test/src/stdio/fdopen_test.cpp
@@ -1,4 +1,4 @@
-//===-- Unittest for fcntl ------------------------------------------------===//
+//===-- Unittest for fdopen -----------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -6,9 +6,9 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "include/llvm-libc-macros/linux/fcntl-macros.h"
 #include "src/stdio/fdopen.h"
 
+#include "include/llvm-libc-macros/fcntl-macros.h"
 #include "src/errno/libc_errno.h"
 #include "src/fcntl/open.h"
 #include "src/stdio/fclose.h"
@@ -37,6 +37,8 @@ TEST(LlvmLibcStdioFdopenTest, WriteAppendRead) {
   constexpr const char LLVM[] = "LLVM";
   int fd2 = LIBC_NAMESPACE::open(TEST_FILE, O_CREAT | O_RDWR);
   auto *fp2 = LIBC_NAMESPACE::fdopen(fd2, "a");
+  ASSERT_ERRNO_SUCCESS();
+  ASSERT_TRUE(nullptr != fp2);
   LIBC_NAMESPACE::fputs(LLVM, fp2);
   LIBC_NAMESPACE::fclose(fp2);
   ASSERT_ERRNO_SUCCESS();

>From 5f70faa6c26fb2c0232542ff142f208892e23fd4 Mon Sep 17 00:00:00 2001
From: Xu Zhang <simonzgx at gmail.com>
Date: Sat, 8 Jun 2024 03:14:47 +0800
Subject: [PATCH 7/9] Move the implementation of 'fdopen' to 'linux/file.cpp'

---
 libc/src/__support/File/file.h         |  2 +
 libc/src/__support/File/linux/file.cpp | 58 ++++++++++++++++++++++-
 libc/src/__support/OSUtil/fcntl.h      |  4 +-
 libc/src/stdio/linux/CMakeLists.txt    |  4 +-
 libc/src/stdio/linux/fdopen.cpp        | 64 ++------------------------
 libc/test/src/stdio/fdopen_test.cpp    |  5 +-
 6 files changed, 67 insertions(+), 70 deletions(-)

diff --git a/libc/src/__support/File/file.h b/libc/src/__support/File/file.h
index 06154871485ce..9acb8d173d076 100644
--- a/libc/src/__support/File/file.h
+++ b/libc/src/__support/File/file.h
@@ -304,6 +304,8 @@ class File {
 // library.
 ErrorOr<File *> openfile(const char *path, const char *mode);
 
+ErrorOr<File *> create_file_from_fd(int fd, const char *mode);
+
 // The platform_file library should implement it if it relevant for that
 // platform.
 int get_fileno(File *f);
diff --git a/libc/src/__support/File/linux/file.cpp b/libc/src/__support/File/linux/file.cpp
index b84da64cbe635..f1453c09807d1 100644
--- a/libc/src/__support/File/linux/file.cpp
+++ b/libc/src/__support/File/linux/file.cpp
@@ -8,10 +8,10 @@
 
 #include "file.h"
 
-#include "src/__support/File/file.h"
-
 #include "src/__support/CPP/new.h"
+#include "src/__support/File/file.h"
 #include "src/__support/File/linux/lseekImpl.h"
+#include "src/__support/OSUtil/fcntl.h"
 #include "src/__support/OSUtil/syscall.h" // For internal syscall function.
 #include "src/errno/libc_errno.h"         // For error macros
 
@@ -119,6 +119,60 @@ ErrorOr<File *> openfile(const char *path, const char *mode) {
   return file;
 }
 
+ErrorOr<File *> create_file_from_fd(int fd, const char *mode) {
+  using ModeFlags = File::ModeFlags;
+  ModeFlags modeflags = File::mode_flags(mode);
+  if (modeflags == 0) {
+    return Error(EINVAL);
+  }
+
+  int fd_flags = internal::fcntl(fd, F_GETFL);
+  if (fd_flags == -1) {
+    return Error(EBADF);
+  }
+
+  using OpenMode = File::OpenMode;
+  if (((fd_flags & O_ACCMODE) == O_RDONLY &&
+       !(modeflags & static_cast<ModeFlags>(OpenMode::READ))) ||
+      ((fd_flags & O_ACCMODE) == O_WRONLY &&
+       !(modeflags & static_cast<ModeFlags>(OpenMode::WRITE)))) {
+    return Error(EINVAL);
+  }
+
+  bool do_seek = false;
+  if ((modeflags & static_cast<ModeFlags>(OpenMode::APPEND)) &&
+      !(fd_flags & O_APPEND)) {
+    do_seek = true;
+    if (internal::fcntl(fd, F_SETFL,
+                        reinterpret_cast<void *>(fd_flags | O_APPEND)) == -1) {
+      return Error(EBADF);
+    }
+  }
+
+  uint8_t *buffer;
+  {
+    AllocChecker ac;
+    buffer = new (ac) uint8_t[File::DEFAULT_BUFFER_SIZE];
+    if (!ac) {
+      return Error(ENOMEM);
+    }
+  }
+  AllocChecker ac;
+  auto *file = new (ac)
+      LinuxFile(fd, buffer, File::DEFAULT_BUFFER_SIZE, _IOFBF, true, modeflags);
+  if (!ac) {
+    return Error(ENOMEM);
+  }
+  if (do_seek) {
+    auto result = file->seek(0, SEEK_END);
+    if (!result.has_value()) {
+      free(file);
+      return Error(result.error());
+    }
+  }
+  return file;
+}
+
 int get_fileno(File *f) {
   auto *lf = reinterpret_cast<LinuxFile *>(f);
   return lf->get_fd();
diff --git a/libc/src/__support/OSUtil/fcntl.h b/libc/src/__support/OSUtil/fcntl.h
index 9121576d57c86..d934545c28d76 100644
--- a/libc/src/__support/OSUtil/fcntl.h
+++ b/libc/src/__support/OSUtil/fcntl.h
@@ -1,6 +1,4 @@
-//===-- Implementation header of internal fcntl function ---------------*- C++
-//-*-===//
-//
+//===-- Implementation header of internal fcntl function ------------------===//
 // 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
diff --git a/libc/src/stdio/linux/CMakeLists.txt b/libc/src/stdio/linux/CMakeLists.txt
index 80998f0d4bfa9..fa36732a159be 100644
--- a/libc/src/stdio/linux/CMakeLists.txt
+++ b/libc/src/stdio/linux/CMakeLists.txt
@@ -33,8 +33,6 @@ add_entrypoint_object(
     ../fdopen.h
   DEPENDS
     libc.include.stdio
-    libc.src.errno.errno
-    libc.src.stdio.fseek
+    libc.src.__support.File.file
     libc.src.__support.File.platform_file
-    libc.src.__support.OSUtil.osutil
 )
diff --git a/libc/src/stdio/linux/fdopen.cpp b/libc/src/stdio/linux/fdopen.cpp
index 48fc3f880becb..71d378d69c417 100644
--- a/libc/src/stdio/linux/fdopen.cpp
+++ b/libc/src/stdio/linux/fdopen.cpp
@@ -8,72 +8,18 @@
 
 #include "src/stdio/fdopen.h"
 
-#include "hdr/errno_macros.h"
-#include "include/llvm-libc-macros/fcntl-macros.h"
-#include "src/__support/File/linux/file.h"
-#include "src/__support/OSUtil/fcntl.h"
+#include "src/__support/File/file.h"
 #include "src/errno/libc_errno.h"
 
 namespace LIBC_NAMESPACE {
 
 LLVM_LIBC_FUNCTION(::FILE *, fdopen, (int fd, const char *mode)) {
-  using ModeFlags = File::ModeFlags;
-  ModeFlags modeflags = File::mode_flags(mode);
-  if (modeflags == 0) {
-    libc_errno = EINVAL;
+  auto result = LIBC_NAMESPACE::create_file_from_fd(fd, mode);
+  if (!result.has_value()) {
+    libc_errno = result.error();
     return nullptr;
   }
-
-  int fd_flags = internal::fcntl(fd, F_GETFL);
-  if (fd_flags == -1) {
-    return nullptr;
-  }
-
-  using OpenMode = File::OpenMode;
-  if (((fd_flags & O_ACCMODE) == O_RDONLY &&
-       !(modeflags & static_cast<ModeFlags>(OpenMode::READ))) ||
-      ((fd_flags & O_ACCMODE) == O_WRONLY &&
-       !(modeflags & static_cast<ModeFlags>(OpenMode::WRITE)))) {
-    libc_errno = EINVAL;
-    return nullptr;
-  }
-
-  bool do_seek = false;
-  if ((modeflags & static_cast<ModeFlags>(OpenMode::APPEND)) &&
-      !(fd_flags & O_APPEND)) {
-    do_seek = true;
-    if (internal::fcntl(fd, F_SETFL,
-                        reinterpret_cast<void *>(fd_flags | O_APPEND)) == -1) {
-      return nullptr;
-    }
-  }
-
-  uint8_t *buffer;
-  {
-    AllocChecker ac;
-    buffer = new (ac) uint8_t[File::DEFAULT_BUFFER_SIZE];
-    if (!ac) {
-      libc_errno = ENOMEM;
-      return nullptr;
-    }
-  }
-  AllocChecker ac;
-  auto *file = new (ac)
-      LinuxFile(fd, buffer, File::DEFAULT_BUFFER_SIZE, _IOFBF, true, modeflags);
-  if (!ac) {
-    libc_errno = ENOMEM;
-    return nullptr;
-  }
-  if (do_seek) {
-    auto result = file->seek(0, SEEK_END);
-    if (!result.has_value()) {
-      libc_errno = result.error();
-      free(file);
-      return nullptr;
-    }
-  }
-
-  return reinterpret_cast<::FILE *>(file);
+  return reinterpret_cast<::FILE *>(result.value());
 }
 
 } // namespace LIBC_NAMESPACE
diff --git a/libc/test/src/stdio/fdopen_test.cpp b/libc/test/src/stdio/fdopen_test.cpp
index 80e249e9d8dce..ef36cff2ffbd5 100644
--- a/libc/test/src/stdio/fdopen_test.cpp
+++ b/libc/test/src/stdio/fdopen_test.cpp
@@ -8,7 +8,7 @@
 
 #include "src/stdio/fdopen.h"
 
-#include "include/llvm-libc-macros/fcntl-macros.h"
+#include "hdr/fcntl_macros.h"
 #include "src/errno/libc_errno.h"
 #include "src/fcntl/open.h"
 #include "src/stdio/fclose.h"
@@ -47,8 +47,7 @@ TEST(LlvmLibcStdioFdopenTest, WriteAppendRead) {
   auto *fp3 = LIBC_NAMESPACE::fdopen(fd3, "r");
   char buffer[10];
   LIBC_NAMESPACE::fgets(buffer, sizeof(buffer), fp3);
-  EXPECT_EQ('H', buffer[0]);
-  EXPECT_EQ('M', buffer[8]);
+  ASSERT_STREQ("HelloLLVM", buffer);
   LIBC_NAMESPACE::fclose(fp3);
   ASSERT_ERRNO_SUCCESS();
 }

>From 8203762952eccac84c2bea31bfaa8349bdad7801 Mon Sep 17 00:00:00 2001
From: Xu Zhang <simonzgx at gmail.com>
Date: Sat, 8 Jun 2024 03:36:04 +0800
Subject: [PATCH 8/9] Add comment for create_file_from_fd

---
 libc/src/__support/File/file.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/libc/src/__support/File/file.h b/libc/src/__support/File/file.h
index 9acb8d173d076..9c9fdfdc25ab1 100644
--- a/libc/src/__support/File/file.h
+++ b/libc/src/__support/File/file.h
@@ -300,10 +300,10 @@ class File {
   }
 };
 
-// The implementaiton of this function is provided by the platform_file
-// library.
+// The implementation of the following functions is provided by the
+// platform_file library.
 ErrorOr<File *> openfile(const char *path, const char *mode);
-
+// Create a File object and associate it with a fd. 
 ErrorOr<File *> create_file_from_fd(int fd, const char *mode);
 
 // The platform_file library should implement it if it relevant for that

>From 07c232956834cd1572dd661ad95c9e0f2d8f2d43 Mon Sep 17 00:00:00 2001
From: Xu Zhang <simonzgx at gmail.com>
Date: Sat, 8 Jun 2024 03:41:59 +0800
Subject: [PATCH 9/9] Format code

---
 libc/src/__support/File/file.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libc/src/__support/File/file.h b/libc/src/__support/File/file.h
index 9c9fdfdc25ab1..09b6a9d037ac5 100644
--- a/libc/src/__support/File/file.h
+++ b/libc/src/__support/File/file.h
@@ -303,7 +303,7 @@ class File {
 // The implementation of the following functions is provided by the
 // platform_file library.
 ErrorOr<File *> openfile(const char *path, const char *mode);
-// Create a File object and associate it with a fd. 
+// Create a File object and associate it with a fd.
 ErrorOr<File *> create_file_from_fd(int fd, const char *mode);
 
 // The platform_file library should implement it if it relevant for that



More information about the libc-commits mailing list