[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 09:28:27 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/6] [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/6] 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/6] 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/6] 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/6] 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/6] 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();
More information about the libc-commits
mailing list