[libc] Return errno from OFD failure paths in fcntl. #166252
Merged
+94
−73
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: Jackson Stogel (jtstogel) ChangesFull diff: https://github.com/llvm/llvm-project/pull/166252.diff 1 Files Affected:
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 ||
|
|
Do we have a test for this? |
jtstogel
changed the title
There's an analogous test that checks llvm-project/libc/test/src/fcntl/fcntl_test.cpp Lines 160 to 168 in 285b57b It looks like more generally none of the OFD paths are tested. Do we have parameterized testing for libc or an issue to add something like |
@lntue to confirm, but I think we basically do ad-hoc parametrized testing in various places -- definitely for math functions with various rounding modes, but also for some stdlib routines: llvm-project/libc/test/src/stdlib/StrtolTest.h Lines 421 to 423 in 285b57b |
+1 for small ad-hoc parametrizing tests when it makes sense, like when the behaviors / implementations won't diverge in the future. |
jtstogel
changed the title
lntue
reviewed
Nov 5, 2025
vonosmas
approved these changes
Nov 5, 2025
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/131/builds/33686 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/147/builds/32458 Here is the relevant piece of the build log for the reference |
jtstogel
added a commit
that referenced
this pull request
Nov 5, 2025
Reverts #166252 Causing buildbot failures on `libc-x86_64-debian-dbg-asan`.
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Nov 5, 2025
…l." (#166658) Reverts llvm/llvm-project#166252 Causing buildbot failures on `libc-x86_64-debian-dbg-asan`.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
This patch also configures fcntl lock tests to run with F_OFD_* command variants, as all existing lock tests do not exercise process-associated- or OFD-specific functionality.