[libc] Implement fchown#167286
Conversation
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
@llvm/pr-subscribers-libc Author: Anshul Nigham (nigham) ChangesImplements fchown fixes: #166856 Full diff: https://github.com/llvm/llvm-project/pull/167286.diff 9 Files Affected:
diff --git a/libc/config/linux/aarch64/entrypoints.txt b/libc/config/linux/aarch64/entrypoints.txt
index e0dd15b803253..144237aee7f93 100644
--- a/libc/config/linux/aarch64/entrypoints.txt
+++ b/libc/config/linux/aarch64/entrypoints.txt
@@ -327,6 +327,7 @@ set(TARGET_LIBC_ENTRYPOINTS
libc.src.unistd.execve
libc.src.unistd.faccessat
libc.src.unistd.fchdir
+ libc.src.unistd.fchown
libc.src.unistd.fpathconf
libc.src.unistd.fsync
libc.src.unistd.ftruncate
diff --git a/libc/config/linux/x86_64/entrypoints.txt b/libc/config/linux/x86_64/entrypoints.txt
index a44e2041e57f2..f4012514fe20e 100644
--- a/libc/config/linux/x86_64/entrypoints.txt
+++ b/libc/config/linux/x86_64/entrypoints.txt
@@ -334,6 +334,7 @@ set(TARGET_LIBC_ENTRYPOINTS
libc.src.unistd.execve
libc.src.unistd.faccessat
libc.src.unistd.fchdir
+ libc.src.unistd.fchown
libc.src.unistd.fpathconf
libc.src.unistd.fsync
libc.src.unistd.ftruncate
diff --git a/libc/include/unistd.yaml b/libc/include/unistd.yaml
index 0e5b22e627b67..3f5e957768533 100644
--- a/libc/include/unistd.yaml
+++ b/libc/include/unistd.yaml
@@ -120,6 +120,14 @@ functions:
return_type: int
arguments:
- type: int
+ - name: fchown
+ standards:
+ - POSIX
+ return_type: int
+ arguments:
+ - type: int
+ - type: uid_t
+ - type: gid_t
- name: fork
standards:
- POSIX
diff --git a/libc/src/unistd/CMakeLists.txt b/libc/src/unistd/CMakeLists.txt
index 337480cbbf928..b7444a4722b0d 100644
--- a/libc/src/unistd/CMakeLists.txt
+++ b/libc/src/unistd/CMakeLists.txt
@@ -76,6 +76,13 @@ add_entrypoint_object(
.${LIBC_TARGET_OS}.fchdir
)
+add_entrypoint_object(
+ fchown
+ ALIAS
+ DEPENDS
+ .${LIBC_TARGET_OS}.fchown
+)
+
add_entrypoint_object(
fork
ALIAS
diff --git a/libc/src/unistd/fchown.h b/libc/src/unistd/fchown.h
new file mode 100644
index 0000000000000..575f6173e980f
--- /dev/null
+++ b/libc/src/unistd/fchown.h
@@ -0,0 +1,22 @@
+//===-- Implementation header for fchown ------------------------*- 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_UNISTD_FCHOWN_H_
+#define LLVM_LIBC_SRC_UNISTD_FCHOWN_H_
+
+#include "hdr/types/gid_t.h"
+#include "hdr/types/uid_t.h"
+#include "src/__support/macros/config.h"
+
+namespace LIBC_NAMESPACE_DECL {
+
+int fchown(int fildes, uid_t owner, gid_t group);
+
+} // namespace LIBC_NAMESPACE_DECL
+
+#endif // LLVM_LIBC_SRC_UNISTD_FCHOWN_H_
diff --git a/libc/src/unistd/linux/CMakeLists.txt b/libc/src/unistd/linux/CMakeLists.txt
index c2dacc6456e27..c45b6ef1c5d80 100644
--- a/libc/src/unistd/linux/CMakeLists.txt
+++ b/libc/src/unistd/linux/CMakeLists.txt
@@ -120,6 +120,20 @@ add_entrypoint_object(
libc.src.errno.errno
)
+add_entrypoint_object(
+ fchown
+ SRCS
+ fchown.cpp
+ HDRS
+ ../fchown.h
+ DEPENDS
+ libc.hdr.types.uid_t
+ libc.hdr.types.gid_t
+ libc.include.sys_syscall
+ libc.src.__support.OSUtil.osutil
+ libc.src.errno.errno
+)
+
add_entrypoint_object(
fork
SRCS
diff --git a/libc/src/unistd/linux/fchown.cpp b/libc/src/unistd/linux/fchown.cpp
new file mode 100644
index 0000000000000..63cad81ea585b
--- /dev/null
+++ b/libc/src/unistd/linux/fchown.cpp
@@ -0,0 +1,29 @@
+//===-- Linux implementation of fchown ------------------------------------===//
+//
+// 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/unistd/fchown.h"
+
+#include "src/__support/OSUtil/syscall.h" // For internal syscall function.
+#include "src/__support/common.h"
+
+#include "src/__support/libc_errno.h"
+#include "src/__support/macros/config.h"
+#include <sys/syscall.h> // For syscall numbers.
+
+namespace LIBC_NAMESPACE_DECL {
+
+LLVM_LIBC_FUNCTION(int, fchown, (int fildes, uid_t owner, gid_t group)) {
+ int ret = LIBC_NAMESPACE::syscall_impl<int>(SYS_fchown, fildes, owner, group);
+ if (ret < 0) {
+ libc_errno = -ret;
+ return -1;
+ }
+ return 0;
+}
+
+} // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/test/src/unistd/CMakeLists.txt b/libc/test/src/unistd/CMakeLists.txt
index 07070535459ec..3012ea9a466f4 100644
--- a/libc/test/src/unistd/CMakeLists.txt
+++ b/libc/test/src/unistd/CMakeLists.txt
@@ -146,6 +146,26 @@ add_libc_unittest(
libc.test.UnitTest.ErrnoSetterMatcher
)
+add_libc_unittest(
+ fchown_test
+ SUITE
+ libc_unistd_unittests
+ SRCS
+ fchown_test.cpp
+ DEPENDS
+ libc.hdr.fcntl_macros
+ libc.include.unistd
+ libc.src.errno.errno
+ libc.src.unistd.fchown
+ libc.src.unistd.close
+ libc.src.unistd.unlink
+ libc.src.fcntl.open
+ libc.src.unistd.getuid
+ libc.src.unistd.getgid
+ libc.test.UnitTest.ErrnoCheckingTest
+ libc.test.UnitTest.ErrnoSetterMatcher
+)
+
add_libc_unittest(
ftruncate_test
SUITE
diff --git a/libc/test/src/unistd/fchown_test.cpp b/libc/test/src/unistd/fchown_test.cpp
new file mode 100644
index 0000000000000..868814e503be6
--- /dev/null
+++ b/libc/test/src/unistd/fchown_test.cpp
@@ -0,0 +1,50 @@
+//===-- Unittests for fchown ----------------------------------------------===//
+//
+// 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/fcntl/open.h"
+#include "src/unistd/close.h"
+#include "src/unistd/fchown.h"
+#include "src/unistd/getgid.h"
+#include "src/unistd/getuid.h"
+#include "src/unistd/unlink.h"
+
+#include "test/UnitTest/ErrnoCheckingTest.h"
+#include "test/UnitTest/ErrnoSetterMatcher.h"
+#include "test/UnitTest/Test.h"
+
+#include "hdr/fcntl_macros.h"
+#include <sys/stat.h>
+
+using LlvmLibcFchownTest = LIBC_NAMESPACE::testing::ErrnoCheckingTest;
+
+TEST_F(LlvmLibcFchownTest, FchownSuccess) {
+ using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Succeeds;
+ uid_t my_uid = LIBC_NAMESPACE::getuid();
+ gid_t my_gid = LIBC_NAMESPACE::getgid();
+ constexpr const char *FILENAME = "fchown.test";
+ auto TEST_FILE = libc_make_test_file_path(FILENAME);
+
+ // Create a test file.
+ int write_fd = LIBC_NAMESPACE::open(TEST_FILE, O_WRONLY | O_CREAT, S_IRWXU);
+ ASSERT_ERRNO_SUCCESS();
+ ASSERT_GT(write_fd, 0);
+
+ // Change the ownership of the file.
+ ASSERT_THAT(LIBC_NAMESPACE::fchown(write_fd, my_uid, my_gid), Succeeds(0));
+
+ // Close the file descriptor.
+ ASSERT_THAT(LIBC_NAMESPACE::close(write_fd), Succeeds(0));
+
+ // Clean up the test file.
+ ASSERT_THAT(LIBC_NAMESPACE::unlink(TEST_FILE), Succeeds(0));
+}
+
+TEST_F(LlvmLibcFchownTest, ChownInvalidFileDescriptor) {
+ using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Fails;
+ ASSERT_THAT(LIBC_NAMESPACE::fchown(-1, 1000, 1000), Fails(EBADF));
+}
|
michaelrj-google
left a comment
There was a problem hiding this comment.
Overall looks good, a couple minor nits.
|
|
||
| #include "src/__support/OSUtil/syscall.h" // For internal syscall function. | ||
| #include "src/__support/common.h" | ||
|
|
There was a problem hiding this comment.
add includes for the proxy headers here, ideally we'd include everything we use.
| #include "hdr/types/gid_t.h" | |
| #include "hdr/types/uid_t.h" |
libc/src/unistd/fchown.h
Outdated
| // | ||
| //===----------------------------------------------------------------------===// | ||
|
|
||
| #ifndef LLVM_LIBC_SRC_UNISTD_FCHOWN_H_ |
There was a problem hiding this comment.
these header guards have an extra underscore at the end that they don't need.
| #ifndef LLVM_LIBC_SRC_UNISTD_FCHOWN_H_ | |
| #ifndef LLVM_LIBC_SRC_UNISTD_FCHOWN_H |
libc/test/src/unistd/fchown_test.cpp
Outdated
| ASSERT_THAT(LIBC_NAMESPACE::unlink(TEST_FILE), Succeeds(0)); | ||
| } | ||
|
|
||
| TEST_F(LlvmLibcFchownTest, ChownInvalidFileDescriptor) { |
There was a problem hiding this comment.
nit: Fix the test name
michaelrj-google
left a comment
There was a problem hiding this comment.
LGTM, you might want to add Fixes #<issue number> to the commit description on github to automatically close the issue when you merge.
Implements fchown
fixes: #166856