[libc] implement sys/uio/readv
#124718
Merged
[libc] implement sys/uio/readv
#124718
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-libc Author: None (c8ef) ChangesCloses #124694. Full diff: https://github.com/llvm/llvm-project/pull/124718.diff 9 Files Affected:
diff --git a/libc/config/linux/aarch64/entrypoints.txt b/libc/config/linux/aarch64/entrypoints.txt
index 09eb51a3f8fc62..99b1187b43e11b 100644
--- a/libc/config/linux/aarch64/entrypoints.txt
+++ b/libc/config/linux/aarch64/entrypoints.txt
@@ -355,6 +355,7 @@ set(TARGET_LIBC_ENTRYPOINTS
# sys/uio.h entrypoints
libc.src.sys.uio.writev
+ libc.src.sys.uio.readv
)
if(LLVM_LIBC_INCLUDE_SCUDO)
diff --git a/libc/config/linux/x86_64/entrypoints.txt b/libc/config/linux/x86_64/entrypoints.txt
index 366e4d34294d15..df85f87140b88b 100644
--- a/libc/config/linux/x86_64/entrypoints.txt
+++ b/libc/config/linux/x86_64/entrypoints.txt
@@ -355,6 +355,7 @@ set(TARGET_LIBC_ENTRYPOINTS
# sys/uio.h entrypoints
libc.src.sys.uio.writev
+ libc.src.sys.uio.readv
)
if(LLVM_LIBC_INCLUDE_SCUDO)
diff --git a/libc/include/sys/uio.yaml b/libc/include/sys/uio.yaml
index 87c5bdff48245c..6d3f336b2b5204 100644
--- a/libc/include/sys/uio.yaml
+++ b/libc/include/sys/uio.yaml
@@ -15,3 +15,11 @@ functions:
- type: int
- type: const struct iovec *
- type: int
+ - name: readv
+ standards:
+ - POSIX
+ return_type: ssize_t
+ arguments:
+ - type: int
+ - type: const struct iovec *
+ - type: int
diff --git a/libc/src/sys/uio/CMakeLists.txt b/libc/src/sys/uio/CMakeLists.txt
index 6298f86cd937da..c6a64297904103 100644
--- a/libc/src/sys/uio/CMakeLists.txt
+++ b/libc/src/sys/uio/CMakeLists.txt
@@ -8,3 +8,10 @@ add_entrypoint_object(
DEPENDS
.${LIBC_TARGET_OS}.writev
)
+
+add_entrypoint_object(
+ readv
+ ALIAS
+ DEPENDS
+ .${LIBC_TARGET_OS}.readv
+)
diff --git a/libc/src/sys/uio/linux/CMakeLists.txt b/libc/src/sys/uio/linux/CMakeLists.txt
index 85a7a3ae4d5c21..7901c741f69a5a 100644
--- a/libc/src/sys/uio/linux/CMakeLists.txt
+++ b/libc/src/sys/uio/linux/CMakeLists.txt
@@ -12,3 +12,18 @@ add_entrypoint_object(
libc.hdr.types.ssize_t
libc.hdr.types.struct_iovec
)
+
+add_entrypoint_object(
+ readv
+ SRCS
+ readv.cpp
+ HDRS
+ ../readv.h
+ DEPENDS
+ libc.include.sys_syscall
+ libc.src.__support.OSUtil.osutil
+ libc.src.__support.common
+ libc.src.errno.errno
+ libc.hdr.types.ssize_t
+ libc.hdr.types.struct_iovec
+)
diff --git a/libc/src/sys/uio/linux/readv.cpp b/libc/src/sys/uio/linux/readv.cpp
new file mode 100644
index 00000000000000..3812f4dec150b8
--- /dev/null
+++ b/libc/src/sys/uio/linux/readv.cpp
@@ -0,0 +1,27 @@
+//===-- Implementation file for readv -------------------------------------===//
+//
+// 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/sys/uio/readv.h"
+#include "src/__support/OSUtil/syscall.h"
+#include "src/__support/common.h"
+#include "src/errno/libc_errno.h"
+#include <sys/syscall.h>
+
+namespace LIBC_NAMESPACE_DECL {
+
+LLVM_LIBC_FUNCTION(ssize_t, readv, (int fd, const iovec *iov, int iovcnt)) {
+ long ret = LIBC_NAMESPACE::syscall_impl<long>(SYS_readv, fd, iov, iovcnt);
+ // On failure, return -1 and set errno.
+ if (ret < 0) {
+ libc_errno = static_cast<int>(-ret);
+ return -1;
+ }
+ // On success, return number of bytes read.
+ return static_cast<ssize_t>(ret);
+}
+
+} // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/src/sys/uio/readv.h b/libc/src/sys/uio/readv.h
new file mode 100644
index 00000000000000..135b1e6fd1b5ac
--- /dev/null
+++ b/libc/src/sys/uio/readv.h
@@ -0,0 +1,22 @@
+//===-- Implementation header for readv -----------------------------------===//
+//
+// 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_SYS_UIO_READV_H
+#define LLVM_LIBC_SRC_SYS_UIO_READV_H
+
+#include "hdr/types/ssize_t.h"
+#include "hdr/types/struct_iovec.h"
+#include "src/__support/macros/config.h"
+
+namespace LIBC_NAMESPACE_DECL {
+
+ssize_t readv(int fd, const iovec *iov, int iovcnt);
+
+} // namespace LIBC_NAMESPACE_DECL
+
+#endif // LLVM_LIBC_SRC_SYS_UIO_READV_H
diff --git a/libc/test/src/sys/uio/CMakeLists.txt b/libc/test/src/sys/uio/CMakeLists.txt
index 45f8d14c161792..a58337ec5f01a5 100644
--- a/libc/test/src/sys/uio/CMakeLists.txt
+++ b/libc/test/src/sys/uio/CMakeLists.txt
@@ -1,8 +1,9 @@
add_custom_target(libc_sys_uio_unittests)
+
add_libc_unittest(
writev_test
SUITE
- libc_sys_uio_unittests
+ libc_sys_uio_unittests
SRCS
writev_test.cpp
DEPENDS
@@ -13,3 +14,18 @@ add_libc_unittest(
libc.src.fcntl.open
libc.test.UnitTest.ErrnoSetterMatcher
)
+
+add_libc_unittest(
+ readv_test
+ SUITE
+ libc_sys_uio_unittests
+ SRCS
+ readv_test.cpp
+ DEPENDS
+ libc.src.errno.errno
+ libc.src.__support.common
+ libc.src.sys.uio.readv
+ libc.src.unistd.close
+ libc.src.fcntl.open
+ libc.test.UnitTest.ErrnoSetterMatcher
+)
diff --git a/libc/test/src/sys/uio/readv_test.cpp b/libc/test/src/sys/uio/readv_test.cpp
new file mode 100644
index 00000000000000..cf551d364653bc
--- /dev/null
+++ b/libc/test/src/sys/uio/readv_test.cpp
@@ -0,0 +1,30 @@
+//===-- Unittests for readv -----------------------------------------------===//
+//
+// 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/sys/uio/readv.h"
+#include "src/unistd/close.h"
+#include "test/UnitTest/ErrnoSetterMatcher.h"
+#include "test/UnitTest/Test.h"
+
+using namespace LIBC_NAMESPACE::testing::ErrnoSetterMatcher;
+
+TEST(LlvmLibcSysUioReadvTest, SmokeTest) {
+ int fd = LIBC_NAMESPACE::open("/dev/urandom", O_RDONLY);
+ ASSERT_THAT(fd, returns(GT(0)).with_errno(EQ(0)));
+ char buf0[2];
+ char buf1[3];
+ struct iovec iov[2];
+ iov[0].iov_base = buf0;
+ iov[0].iov_len = 1;
+ iov[1].iov_base = buf1;
+ iov[1].iov_len = 2;
+ ASSERT_THAT(LIBC_NAMESPACE::readv(fd, iov, 2),
+ returns(EQ(3)).with_errno(EQ(0)));
+ ASSERT_THAT(LIBC_NAMESPACE::close(fd), Succeeds());
+}
|
nickdesaulniers
approved these changes
Jan 28, 2025
Comment on lines
23
to
28
| libc.include.sys_syscall | ||
| libc.src.__support.OSUtil.osutil | ||
| libc.src.__support.common | ||
| libc.src.errno.errno | ||
| libc.hdr.types.ssize_t | ||
| libc.hdr.types.struct_iovec |
There was a problem hiding this comment.
sort alphabetically, please
libc/test/src/sys/uio/readv_test.cpp
Outdated
| using namespace LIBC_NAMESPACE::testing::ErrnoSetterMatcher; | ||
|
|
||
| TEST(LlvmLibcSysUioReadvTest, SmokeTest) { | ||
| int fd = LIBC_NAMESPACE::open("/dev/urandom", O_RDONLY); |
There was a problem hiding this comment.
I see now that the existing libc/test/src/sys/uio/writev_test.cpp also opens /dev/random...but doesn't that mean these posix mandated functions won't run on windows? cc @SchrodingerZhu
There was a problem hiding this comment.
Maybe it would be more convenient to use an API like tmpfile? Or should we write to a file first and then read from it?
There was a problem hiding this comment.
Yeah, I think writing a sequence and reading it back is a better test.
There was a problem hiding this comment.
Ok, let's get this fixed up first before landing then, please. Or at least file a bug saying this is an issue for the writev_test and readv_test, then add a link to that issue in a comment in both test files in this PR.
There was a problem hiding this comment.
Done. Switch to using local files.
Done. I set it to 1024 here since the default value for glibc and musl is 1024. |
|
Gentle ping~ @nickdesaulniers |
|
Friendly ping! |
nickdesaulniers
approved these changes
Feb 5, 2025
There was a problem hiding this comment.
Thanks for fixing up the pre-existing test, too! That's above and beyond the call of duty.
We might want to use EXPECT_THAT, such that we always call unlink even if prior assertions fail, but open with O_CREAT may make that less of an issue. It might be more helpful for us one day to register an at exit helper to clean up any temp files, for use from unit tests.
Using an exit helper is a great idea. I will remember to use this approach in the future! |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/153/builds/22071 Here is the relevant piece of the build log for the reference |
Icohedron
pushed a commit
to Icohedron/llvm-project
that referenced
this pull request
Feb 11, 2025
Closes llvm#124694. This patch adds the `sys/uio/readv` function. ref: https://pubs.opengroup.org/onlinepubs/009696699/functions/readv.html
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #124694.
This patch adds the
sys/uio/readvfunction.ref: https://pubs.opengroup.org/onlinepubs/009696699/functions/readv.html