[libc-commits] [libc] Reapply "[libc] Return errno from OFD failure paths in fcntl." (#166658) (PR #166846)
Jackson Stogel via libc-commits
libc-commits at lists.llvm.org
Thu Nov 6 13:33:08 PST 2025
https://github.com/jtstogel updated https://github.com/llvm/llvm-project/pull/166846
>From 40a8a2b50dcf147c2623c71c9c925143f45c399c Mon Sep 17 00:00:00 2001
From: Jackson Stogel <jtstogel at gmail.com>
Date: Wed, 5 Nov 2025 23:51:31 +0000
Subject: [PATCH 1/3] Reapply "[libc] Return errno from OFD failure paths in
fcntl." (#166658)
This reverts commit 597cd767d6ad2cca0d8676888c40cbc5700db1ca.
---
libc/src/__support/OSUtil/linux/fcntl.cpp | 2 +-
libc/test/src/fcntl/fcntl_test.cpp | 165 ++++++++++++----------
2 files changed, 94 insertions(+), 73 deletions(-)
diff --git a/libc/src/__support/OSUtil/linux/fcntl.cpp b/libc/src/__support/OSUtil/linux/fcntl.cpp
index bb76eee90efd2..08db4859c6417 100644
--- a/libc/src/__support/OSUtil/linux/fcntl.cpp
+++ b/libc/src/__support/OSUtil/linux/fcntl.cpp
@@ -66,7 +66,7 @@ ErrorOr<int> fcntl(int fd, int cmd, void *arg) {
LIBC_NAMESPACE::syscall_impl<int>(FCNTL_SYSCALL_ID, fd, cmd, &flk64);
// On failure, return
if (ret < 0)
- return Error(-1);
+ return Error(-ret);
// 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 ||
diff --git a/libc/test/src/fcntl/fcntl_test.cpp b/libc/test/src/fcntl/fcntl_test.cpp
index 84feb34e537a0..d6aaccddd76ee 100644
--- a/libc/test/src/fcntl/fcntl_test.cpp
+++ b/libc/test/src/fcntl/fcntl_test.cpp
@@ -94,78 +94,99 @@ TEST_F(LlvmLibcFcntlTest, FcntlSetFl) {
ASSERT_THAT(LIBC_NAMESPACE::close(fd), Succeeds(0));
}
-TEST_F(LlvmLibcFcntlTest, FcntlGetLkRead) {
- using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Succeeds;
- constexpr const char *TEST_FILE_NAME = "testdata/fcntl_getlkread.test";
- auto TEST_FILE = libc_make_test_file_path(TEST_FILE_NAME);
-
- struct flock flk, svflk;
- int retVal;
- int fd =
- LIBC_NAMESPACE::open(TEST_FILE, O_CREAT | O_TRUNC | O_RDONLY, S_IRWXU);
- ASSERT_ERRNO_SUCCESS();
- ASSERT_GT(fd, 0);
-
- flk.l_type = F_RDLCK;
- flk.l_start = 0;
- flk.l_whence = SEEK_SET;
- flk.l_len = 50;
-
- // copy flk into svflk
- svflk = flk;
-
- retVal = LIBC_NAMESPACE::fcntl(fd, F_GETLK, &svflk);
- ASSERT_ERRNO_SUCCESS();
- ASSERT_GT(retVal, -1);
- ASSERT_NE((int)flk.l_type, F_WRLCK); // File should not be write locked.
-
- retVal = LIBC_NAMESPACE::fcntl(fd, F_SETLK, &svflk);
- ASSERT_ERRNO_SUCCESS();
- ASSERT_GT(retVal, -1);
-
- ASSERT_THAT(LIBC_NAMESPACE::close(fd), Succeeds(0));
-}
-
-TEST_F(LlvmLibcFcntlTest, FcntlGetLkWrite) {
- using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Succeeds;
- constexpr const char *TEST_FILE_NAME = "testdata/fcntl_getlkwrite.test";
- auto TEST_FILE = libc_make_test_file_path(TEST_FILE_NAME);
-
- struct flock flk, svflk;
- int retVal;
- int fd = LIBC_NAMESPACE::open(TEST_FILE, O_CREAT | O_TRUNC | O_RDWR, S_IRWXU);
- ASSERT_ERRNO_SUCCESS();
- ASSERT_GT(fd, 0);
-
- flk.l_type = F_WRLCK;
- flk.l_start = 0;
- flk.l_whence = SEEK_SET;
- flk.l_len = 0;
-
- // copy flk into svflk
- svflk = flk;
-
- retVal = LIBC_NAMESPACE::fcntl(fd, F_GETLK, &svflk);
- ASSERT_ERRNO_SUCCESS();
- ASSERT_GT(retVal, -1);
- ASSERT_NE((int)flk.l_type, F_RDLCK); // File should not be read locked.
-
- retVal = LIBC_NAMESPACE::fcntl(fd, F_SETLK, &svflk);
- ASSERT_ERRNO_SUCCESS();
- ASSERT_GT(retVal, -1);
-
- ASSERT_THAT(LIBC_NAMESPACE::close(fd), Succeeds(0));
-}
-
-TEST_F(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);
-}
+/* Tests that are common between OFD and traditional variants of fcntl locks. */
+template <int GETLK_CMD, int SETLK_CMD>
+class LibcFcntlCommonLockTests : public LlvmLibcFcntlTest {
+public:
+ void GetLkRead() {
+ using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Succeeds;
+ constexpr const char *TEST_FILE_NAME = "testdata/fcntl_getlkread.test";
+ const auto TEST_FILE = libc_make_test_file_path(TEST_FILE_NAME);
+
+ struct flock flk = {};
+ struct flock svflk = {};
+ int retVal;
+ int fd =
+ LIBC_NAMESPACE::open(TEST_FILE, O_CREAT | O_TRUNC | O_RDONLY, S_IRWXU);
+ ASSERT_ERRNO_SUCCESS();
+ ASSERT_GT(fd, 0);
+
+ flk.l_type = F_RDLCK;
+ flk.l_start = 0;
+ flk.l_whence = SEEK_SET;
+ flk.l_len = 50;
+
+ // copy flk into svflk
+ svflk = flk;
+
+ retVal = LIBC_NAMESPACE::fcntl(fd, GETLK_CMD, &svflk);
+ ASSERT_ERRNO_SUCCESS();
+ ASSERT_GT(retVal, -1);
+ ASSERT_NE((int)svflk.l_type, F_WRLCK); // File should not be write locked.
+
+ retVal = LIBC_NAMESPACE::fcntl(fd, SETLK_CMD, &svflk);
+ ASSERT_ERRNO_SUCCESS();
+ ASSERT_GT(retVal, -1);
+
+ ASSERT_THAT(LIBC_NAMESPACE::close(fd), Succeeds(0));
+ }
+
+ void GetLkWrite() {
+ using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Succeeds;
+ constexpr const char *TEST_FILE_NAME = "testdata/fcntl_getlkwrite.test";
+ const auto TEST_FILE = libc_make_test_file_path(TEST_FILE_NAME);
+
+ struct flock flk = {};
+ struct flock svflk = {};
+ int retVal;
+ int fd =
+ LIBC_NAMESPACE::open(TEST_FILE, O_CREAT | O_TRUNC | O_RDWR, S_IRWXU);
+ ASSERT_ERRNO_SUCCESS();
+ ASSERT_GT(fd, 0);
+
+ flk.l_type = F_WRLCK;
+ flk.l_start = 0;
+ flk.l_whence = SEEK_SET;
+ flk.l_len = 0;
+
+ // copy flk into svflk
+ svflk = flk;
+
+ retVal = LIBC_NAMESPACE::fcntl(fd, GETLK_CMD, &svflk);
+ ASSERT_ERRNO_SUCCESS();
+ ASSERT_GT(retVal, -1);
+ ASSERT_NE((int)svflk.l_type, F_RDLCK); // File should not be read locked.
+
+ retVal = LIBC_NAMESPACE::fcntl(fd, SETLK_CMD, &svflk);
+ ASSERT_ERRNO_SUCCESS();
+ ASSERT_GT(retVal, -1);
+
+ ASSERT_THAT(LIBC_NAMESPACE::close(fd), Succeeds(0));
+ }
+
+ void UseAfterClose() {
+ using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Succeeds;
+ constexpr const char *TEST_FILE_NAME =
+ "testdata/fcntl_use_after_close.test";
+ const 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, GETLK_CMD));
+ ASSERT_ERRNO_EQ(EBADF);
+ }
+};
+
+#define COMMON_LOCK_TESTS(NAME, GETLK, SETLK) \
+ using NAME = LibcFcntlCommonLockTests<GETLK, SETLK>; \
+ TEST_F(NAME, GetLkRead) { GetLkRead(); } \
+ TEST_F(NAME, GetLkWrite) { GetLkWrite(); } \
+ TEST_F(NAME, UseAfterClose) { UseAfterClose(); } \
+ static_assert(true, "Require semicolon.")
+
+COMMON_LOCK_TESTS(LlvmLibcFcntlProcessAssociatedLockTest, F_GETLK, F_SETLK);
+COMMON_LOCK_TESTS(LlvmLibcFcntlOpenFileDescriptionLockTest, F_OFD_GETLK,
+ F_OFD_SETLK);
TEST_F(LlvmLibcFcntlTest, SetGetOwnerTest) {
using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Succeeds;
>From 86f4811624f3b8e278c958ed4b60ed1948ec3157 Mon Sep 17 00:00:00 2001
From: Jackson Stogel <jtstogel at gmail.com>
Date: Thu, 6 Nov 2025 01:41:22 +0000
Subject: [PATCH 2/3] Pass a struct flock into fcntl F_OFD_GETLK.
---
libc/test/src/fcntl/fcntl_test.cpp | 22 +++++++++++++++++++---
1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/libc/test/src/fcntl/fcntl_test.cpp b/libc/test/src/fcntl/fcntl_test.cpp
index d6aaccddd76ee..41a9893d4d902 100644
--- a/libc/test/src/fcntl/fcntl_test.cpp
+++ b/libc/test/src/fcntl/fcntl_test.cpp
@@ -172,13 +172,19 @@ class LibcFcntlCommonLockTests : public LlvmLibcFcntlTest {
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, GETLK_CMD));
+
+ struct flock flk = {};
+ flk.l_type = F_RDLCK;
+ flk.l_start = 0;
+ flk.l_whence = SEEK_SET;
+ flk.l_len = 50;
+ ASSERT_EQ(-1, LIBC_NAMESPACE::fcntl(fd, GETLK_CMD, &flk));
ASSERT_ERRNO_EQ(EBADF);
}
};
-#define COMMON_LOCK_TESTS(NAME, GETLK, SETLK) \
- using NAME = LibcFcntlCommonLockTests<GETLK, SETLK>; \
+#define COMMON_LOCK_TESTS(NAME, GETLK_CMD, SETLK_CMD) \
+ using NAME = LibcFcntlCommonLockTests<GETLK_CMD, GETLK_CMD>; \
TEST_F(NAME, GetLkRead) { GetLkRead(); } \
TEST_F(NAME, GetLkWrite) { GetLkWrite(); } \
TEST_F(NAME, UseAfterClose) { UseAfterClose(); } \
@@ -188,6 +194,16 @@ COMMON_LOCK_TESTS(LlvmLibcFcntlProcessAssociatedLockTest, F_GETLK, F_SETLK);
COMMON_LOCK_TESTS(LlvmLibcFcntlOpenFileDescriptionLockTest, F_OFD_GETLK,
F_OFD_SETLK);
+TEST_F(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);
+}
+
TEST_F(LlvmLibcFcntlTest, SetGetOwnerTest) {
using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Succeeds;
pid_t pid = LIBC_NAMESPACE::getpid();
>From af9df0c654a74a30b6be3b2177a68675c496d366 Mon Sep 17 00:00:00 2001
From: Jackson Stogel <jtstogel at gmail.com>
Date: Thu, 6 Nov 2025 21:32:34 +0000
Subject: [PATCH 3/3] Fix typo.
---
libc/test/src/fcntl/fcntl_test.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libc/test/src/fcntl/fcntl_test.cpp b/libc/test/src/fcntl/fcntl_test.cpp
index 41a9893d4d902..6e789b72cf637 100644
--- a/libc/test/src/fcntl/fcntl_test.cpp
+++ b/libc/test/src/fcntl/fcntl_test.cpp
@@ -184,7 +184,7 @@ class LibcFcntlCommonLockTests : public LlvmLibcFcntlTest {
};
#define COMMON_LOCK_TESTS(NAME, GETLK_CMD, SETLK_CMD) \
- using NAME = LibcFcntlCommonLockTests<GETLK_CMD, GETLK_CMD>; \
+ using NAME = LibcFcntlCommonLockTests<GETLK_CMD, SETLK_CMD>; \
TEST_F(NAME, GetLkRead) { GetLkRead(); } \
TEST_F(NAME, GetLkWrite) { GetLkWrite(); } \
TEST_F(NAME, UseAfterClose) { UseAfterClose(); } \
More information about the libc-commits
mailing list