Skip to content

Revert "[libc] Fix readlink tests on 32-bit systems" #97852

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

Merged
merged 1 commit into from
Jul 5, 2024

Conversation

mikhailramalho
Copy link
Member

Reverts #97850 while I investigate the issue

@llvmbot llvmbot added the libc label Jul 5, 2024
@llvmbot
Copy link
Member

llvmbot commented Jul 5, 2024

@llvm/pr-subscribers-libc

Author: Mikhail R. Gadelha (mikhailramalho)

Changes

Reverts llvm/llvm-project#97850 while I investigate the issue


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

3 Files Affected:

  • (modified) libc/test/src/unistd/CMakeLists.txt (-2)
  • (modified) libc/test/src/unistd/readlink_test.cpp (+4-7)
  • (modified) libc/test/src/unistd/readlinkat_test.cpp (+6-9)
diff --git a/libc/test/src/unistd/CMakeLists.txt b/libc/test/src/unistd/CMakeLists.txt
index f4f78b800987d4..de3e8d9ccbb626 100644
--- a/libc/test/src/unistd/CMakeLists.txt
+++ b/libc/test/src/unistd/CMakeLists.txt
@@ -262,7 +262,6 @@ add_libc_unittest(
     libc.include.unistd
     libc.src.errno.errno
     libc.src.unistd.readlink
-    libc.src.string.string_utils
     libc.src.unistd.symlink
     libc.src.unistd.unlink
     libc.src.__support.CPP.string_view
@@ -279,7 +278,6 @@ add_libc_unittest(
     libc.include.fcntl
     libc.include.unistd
     libc.src.errno.errno
-    libc.src.string.string_utils
     libc.src.unistd.readlinkat
     libc.src.unistd.symlink
     libc.src.unistd.unlink
diff --git a/libc/test/src/unistd/readlink_test.cpp b/libc/test/src/unistd/readlink_test.cpp
index 0760850d9bae19..20f3951349118a 100644
--- a/libc/test/src/unistd/readlink_test.cpp
+++ b/libc/test/src/unistd/readlink_test.cpp
@@ -9,7 +9,6 @@
 #include "src/__support/CPP/string_view.h"
 #include "src/errno/libc_errno.h"
 #include "src/unistd/readlink.h"
-#include "src/string/string_utils.h"
 #include "src/unistd/symlink.h"
 #include "src/unistd/unlink.h"
 #include "test/UnitTest/ErrnoSetterMatcher.h"
@@ -31,9 +30,8 @@ TEST(LlvmLibcReadlinkTest, CreateAndUnlink) {
   //   3. Cleanup the symlink created in step #1.
   ASSERT_THAT(LIBC_NAMESPACE::symlink(LINK_VAL, LINK), Succeeds(0));
 
-  char buf[sizeof(FILENAME)];
-  ssize_t len = LIBC_NAMESPACE::readlink(
-      LINK, buf, LIBC_NAMESPACE::internal::string_length(FILENAME));
+  char buf[sizeof(LINK_VAL)];
+  ssize_t len = LIBC_NAMESPACE::readlink(LINK, buf, sizeof(buf));
   ASSERT_ERRNO_SUCCESS();
   ASSERT_EQ(cpp::string_view(buf, len), cpp::string_view(LINK_VAL));
 
@@ -42,8 +40,7 @@ TEST(LlvmLibcReadlinkTest, CreateAndUnlink) {
 
 TEST(LlvmLibcReadlinkTest, ReadlinkInNonExistentPath) {
   using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Fails;
-  constexpr auto len = 8;
-  char buf[len];
-  ASSERT_THAT(LIBC_NAMESPACE::readlink("non-existent-link", buf, len),
+  char buf[8];
+  ASSERT_THAT(LIBC_NAMESPACE::readlink("non-existent-link", buf, sizeof(buf)),
               Fails(ENOENT));
 }
diff --git a/libc/test/src/unistd/readlinkat_test.cpp b/libc/test/src/unistd/readlinkat_test.cpp
index 61e87731c9b9dd..39d81d9ba544a6 100644
--- a/libc/test/src/unistd/readlinkat_test.cpp
+++ b/libc/test/src/unistd/readlinkat_test.cpp
@@ -9,7 +9,6 @@
 #include "src/__support/CPP/string_view.h"
 #include "src/errno/libc_errno.h"
 #include "src/unistd/readlinkat.h"
-#include "src/string/string_utils.h"
 #include "src/unistd/symlink.h"
 #include "src/unistd/unlink.h"
 #include "test/UnitTest/ErrnoSetterMatcher.h"
@@ -33,9 +32,8 @@ TEST(LlvmLibcReadlinkatTest, CreateAndUnlink) {
   //   3. Cleanup the symlink created in step #1.
   ASSERT_THAT(LIBC_NAMESPACE::symlink(LINK_VAL, LINK), Succeeds(0));
 
-  char buf[sizeof(FILENAME)];
-  ssize_t len = LIBC_NAMESPACE::readlinkat(
-      AT_FDCWD, LINK, buf, LIBC_NAMESPACE::internal::string_length(FILENAME));
+  char buf[sizeof(LINK_VAL)];
+  ssize_t len = LIBC_NAMESPACE::readlinkat(AT_FDCWD, LINK, buf, sizeof(buf));
   ASSERT_ERRNO_SUCCESS();
   ASSERT_EQ(cpp::string_view(buf, len), cpp::string_view(LINK_VAL));
 
@@ -44,9 +42,8 @@ TEST(LlvmLibcReadlinkatTest, CreateAndUnlink) {
 
 TEST(LlvmLibcReadlinkatTest, ReadlinkInNonExistentPath) {
   using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Fails;
-  constexpr auto len = 8;
-  char buf[len];
-  ASSERT_THAT(
-      LIBC_NAMESPACE::readlinkat(AT_FDCWD, "non-existent-link", buf, len),
-      Fails(ENOENT));
+  char buf[8];
+  ASSERT_THAT(LIBC_NAMESPACE::readlinkat(AT_FDCWD, "non-existent-link", buf,
+                                         sizeof(buf)),
+              Fails(ENOENT));
 }

@mikhailramalho mikhailramalho merged commit fc66543 into main Jul 5, 2024
6 of 7 checks passed
@mikhailramalho mikhailramalho deleted the revert-97850-fix-readlink branch July 5, 2024 18:32
kbluck pushed a commit to kbluck/llvm-project that referenced this pull request Jul 6, 2024
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.

2 participants