Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[libc][minor] Fix assertion in LlvmLibcFILETest.SimpleFileOperations #126109

Merged
merged 2 commits into from
Feb 6, 2025

Conversation

alanzhao1
Copy link
Contributor

@alanzhao1 alanzhao1 commented Feb 6, 2025

The file descriptor of the first opened file is not necessarily 3, so we change the assertion so that it's >= 0 (i.e. not an error.)

Fixes #126106

The file descriptor of the first opened file is not necessarily 3, so we
change the assertion so that it's >= 3 (i.e. not 0, 1, 2, or -1 which
correspond to stdin, stdout, stderr, or error respectively.)

Fixes llvm#126106
@llvmbot
Copy link
Member

llvmbot commented Feb 6, 2025

@llvm/pr-subscribers-libc

Author: Alan Zhao (alanzhao1)

Changes

The file descriptor of the first opened file is not necessarily 3, so we change the assertion so that it's >= 3 (i.e. not 0, 1, 2, or -1 which correspond to stdin, stdout, stderr, or error respectively.)

Fixes #126106


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

1 Files Affected:

  • (modified) libc/test/src/stdio/fileop_test.cpp (+1-1)
diff --git a/libc/test/src/stdio/fileop_test.cpp b/libc/test/src/stdio/fileop_test.cpp
index 98ead6edd38b471..d12e9379e158e0f 100644
--- a/libc/test/src/stdio/fileop_test.cpp
+++ b/libc/test/src/stdio/fileop_test.cpp
@@ -31,7 +31,7 @@ TEST(LlvmLibcFILETest, SimpleFileOperations) {
   constexpr char FILENAME[] = "testdata/simple_operations.test";
   ::FILE *file = LIBC_NAMESPACE::fopen(FILENAME, "w");
   ASSERT_FALSE(file == nullptr);
-  ASSERT_EQ(LIBC_NAMESPACE::fileno(file), 3);
+  ASSERT_GE(LIBC_NAMESPACE::fileno(file), 3);
   constexpr char CONTENT[] = "1234567890987654321";
   ASSERT_EQ(sizeof(CONTENT) - 1,
             LIBC_NAMESPACE::fwrite(CONTENT, 1, sizeof(CONTENT) - 1, file));

Copy link
Member

@nickdesaulniers nickdesaulniers left a comment

Choose a reason for hiding this comment

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

Thanks for the patch. We can probably further improve this using fstat and lstat, but I will leave that cleanup for another day. This should at least unblock you from running the rest of the test suite. Thanks for the report, and the fix.

@alanzhao1 alanzhao1 merged commit 4c8acbd into llvm:main Feb 6, 2025
12 of 13 checks passed
@alanzhao1 alanzhao1 deleted the bugfix/libc-fd-test branch February 6, 2025 20:57
Icohedron pushed a commit to Icohedron/llvm-project that referenced this pull request Feb 11, 2025
…lvm#126109)

The file descriptor of the first opened file is not necessarily 3, so we
change the assertion so that it's >= 0 (i.e. not an error.)

Fixes llvm#126106
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.

Test LlvmLibcFILETest.SimpleFileOperations fails due to file descriptor assertion
3 participants