Skip to content

Conversation

@jtstogel
Copy link
Contributor

@jtstogel jtstogel commented Nov 4, 2025

Previously, if the open or openat syscalls returned 0 as a (valid) file descriptor, the creat and openat wrappers would erroneously return -1.

@llvmbot llvmbot added the libc label Nov 4, 2025
@jtstogel jtstogel requested a review from vonosmas November 4, 2025 23:29
@llvmbot
Copy link
Member

llvmbot commented Nov 4, 2025

@llvm/pr-subscribers-libc

Author: Jackson Stogel (jtstogel)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/166466.diff

1 Files Affected:

  • (modified) libc/src/fcntl/linux/openat.cpp (+5-5)
diff --git a/libc/src/fcntl/linux/openat.cpp b/libc/src/fcntl/linux/openat.cpp
index b47ad1fb3bb0f..b80abe532e51c 100644
--- a/libc/src/fcntl/linux/openat.cpp
+++ b/libc/src/fcntl/linux/openat.cpp
@@ -32,11 +32,11 @@ LLVM_LIBC_FUNCTION(int, openat, (int dfd, const char *path, int flags, ...)) {
 
   int fd = LIBC_NAMESPACE::syscall_impl<int>(SYS_openat, dfd, path, flags,
                                              mode_flags);
-  if (fd > 0)
-    return fd;
-
-  libc_errno = -fd;
-  return -1;
+  if (fd < 0) {
+    libc_errno = -fd;
+    return -1;
+  }
+  return fd;
 }
 
 } // namespace LIBC_NAMESPACE_DECL

Copy link
Contributor

@vonosmas vonosmas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the fix!

Before committing, lease expand the description a little bit to clarify why the previous behavior was incorrect.

@jtstogel
Copy link
Contributor Author

jtstogel commented Nov 5, 2025

I've updated this patch to also fix creat, which has the same issue.

@jtstogel jtstogel changed the title [libc] Allow openat to return fd 0. [libc] Allow openat and creat to return fd 0. Nov 5, 2025
@jtstogel jtstogel merged commit c3b2849 into llvm:main Nov 5, 2025
18 of 19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants