[libc-commits] [libc] [libc][fcntl] Simplify the handling of the return value from syscall … (PR #96325)

[Xu Zhang via libc-commits]

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

>From b25d759a4d9290feeb7f46e4b24332fa309d95d8 Mon Sep 17 00:00:00 2001
From: Xu Zhang <simonzgx at gmail.com>
Date: Sat, 22 Jun 2024 00:37:42 +0800
Subject: [PATCH 1/2] [libc][fcntl] Simplify the handling of the return value
 from syscall for the F_GETOWN command.

---
 libc/src/__support/OSUtil/linux/fcntl.cpp | 10 +++-------
 libc/test/src/fcntl/CMakeLists.txt        |  1 +
 libc/test/src/fcntl/fcntl_test.cpp        | 19 +++++++++++++++++++
 3 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/libc/src/__support/OSUtil/linux/fcntl.cpp b/libc/src/__support/OSUtil/linux/fcntl.cpp
index b087f898c395d..083deb8c33681 100644
--- a/libc/src/__support/OSUtil/linux/fcntl.cpp
+++ b/libc/src/__support/OSUtil/linux/fcntl.cpp
@@ -67,15 +67,11 @@ int fcntl(int fd, int cmd, void *arg) {
   }
   case F_GETOWN: {
     struct f_owner_ex fex;
-    int retVal =
+    int ret =
         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)
+    if (ret >= 0)
       return fex.type == F_OWNER_PGRP ? -fex.pid : fex.pid;
-
-    libc_errno = -retVal;
+    libc_errno = -ret;
     return -1;
   }
   // The general case
diff --git a/libc/test/src/fcntl/CMakeLists.txt b/libc/test/src/fcntl/CMakeLists.txt
index aae296f074bea..48048b7fe8866 100644
--- a/libc/test/src/fcntl/CMakeLists.txt
+++ b/libc/test/src/fcntl/CMakeLists.txt
@@ -29,6 +29,7 @@ add_libc_unittest(
     libc.src.fcntl.fcntl
     libc.src.fcntl.open
     libc.src.unistd.close
+    libc.src.unistd.getpid
     libc.hdr.types.struct_flock
     libc.hdr.fcntl_macros
     libc.test.UnitTest.ErrnoSetterMatcher
diff --git a/libc/test/src/fcntl/fcntl_test.cpp b/libc/test/src/fcntl/fcntl_test.cpp
index fc909acc1aa0b..a8fc2b246f5c5 100644
--- a/libc/test/src/fcntl/fcntl_test.cpp
+++ b/libc/test/src/fcntl/fcntl_test.cpp
@@ -12,6 +12,7 @@
 #include "src/fcntl/fcntl.h"
 #include "src/fcntl/open.h"
 #include "src/unistd/close.h"
+#include "src/unistd/getpid.h"
 #include "test/UnitTest/ErrnoSetterMatcher.h"
 #include "test/UnitTest/Test.h"
 
@@ -163,3 +164,21 @@ TEST(LlvmLibcFcntlTest, UseAfterClose) {
   ASSERT_EQ(-1, LIBC_NAMESPACE::fcntl(fd, F_GETFL));
   ASSERT_ERRNO_EQ(EBADF);
 }
+
+TEST(LlvmLibcFcntlTest, SetGetOwnerTest) {
+  using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Succeeds;
+  pid_t pid = LIBC_NAMESPACE::getpid();
+  ASSERT_GT(pid, -1);
+  constexpr const char *TEST_FILE_NAME = "testdata/fcntl_set_get_owner.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_ERRNO_SUCCESS();
+  ASSERT_GT(fd, 0);
+  int ret = LIBC_NAMESPACE::fcntl(fd, F_SETOWN, pid);
+  ASSERT_ERRNO_SUCCESS();
+  ASSERT_GT(ret, -1);
+  int ret2 = LIBC_NAMESPACE::fcntl(fd, F_GETOWN);
+  ASSERT_ERRNO_SUCCESS();
+  ASSERT_EQ(ret2, pid);
+  ASSERT_THAT(LIBC_NAMESPACE::close(fd), Succeeds(0));
+}

>From a7663c3dd0ba1fd1391d1f9a91d4998cb4e065b9 Mon Sep 17 00:00:00 2001
From: Xu Zhang <simonzgx at gmail.com>
Date: Sat, 22 Jun 2024 10:53:37 +0800
Subject: [PATCH 2/2] Fix failed unit tests

---
 libc/test/src/fcntl/fcntl_test.cpp | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libc/test/src/fcntl/fcntl_test.cpp b/libc/test/src/fcntl/fcntl_test.cpp
index a8fc2b246f5c5..928e31089f81b 100644
--- a/libc/test/src/fcntl/fcntl_test.cpp
+++ b/libc/test/src/fcntl/fcntl_test.cpp
@@ -163,6 +163,7 @@ TEST(LlvmLibcFcntlTest, UseAfterClose) {
   ASSERT_THAT(LIBC_NAMESPACE::close(fd), Succeeds(0));
   ASSERT_EQ(-1, LIBC_NAMESPACE::fcntl(fd, F_GETFL));
   ASSERT_ERRNO_EQ(EBADF);
+  LIBC_NAMESPACE::libc_errno = 0;
 }
 
 TEST(LlvmLibcFcntlTest, SetGetOwnerTest) {