Skip to content

Conversation

kaladron
Copy link
Contributor

Replace the custom my_streq() helper function with LIBC_NAMESPACE::strcmp(), simplifying the test code and making it more consistent with other integration tests in the codebase.

Changes:

  • Removed 18-line my_streq() helper function
  • Added #include "src/string/strcmp.h"
  • Updated string comparisons:
    • Null checks: Direct comparison with nullptr
    • Equality checks: ASSERT_EQ(strcmp(...), 0)
    • Inequality checks: ASSERT_NE(strcmp(...), 0)
  • Added libc.src.string.strcmp dependency in CMakeLists.txt

This reduces the test file from 49 to 31 lines while maintaining the same test coverage. The strcmp-based approach is cleaner and aligns with the pattern used in the environment_management branch tests (setenv_test, unsetenv_test, putenv_test).

Testing: Verified that all test assertions pass with the new implementation.

… helper

Replace the custom my_streq() helper function with LIBC_NAMESPACE::strcmp(),
simplifying the test code and making it more consistent with other integration
tests in the codebase.

Changes:
- Removed 18-line my_streq() helper function
- Added #include "src/string/strcmp.h"
- Updated string comparisons:
  * Null checks: Direct comparison with nullptr
  * Equality checks: ASSERT_EQ(strcmp(...), 0)
  * Inequality checks: ASSERT_NE(strcmp(...), 0)
- Added libc.src.string.strcmp dependency in CMakeLists.txt

This reduces the test file from 49 to 31 lines while maintaining the same
test coverage. The strcmp-based approach is cleaner and aligns with the
pattern used in the environment_management branch tests (setenv_test,
unsetenv_test, putenv_test).

Testing: Verified that all test assertions pass with the new implementation.
@kaladron kaladron self-assigned this Oct 12, 2025
@llvmbot llvmbot added the libc label Oct 12, 2025
@llvmbot
Copy link
Member

llvmbot commented Oct 12, 2025

@llvm/pr-subscribers-libc

Author: Jeff Bailey (kaladron)

Changes

Replace the custom my_streq() helper function with LIBC_NAMESPACE::strcmp(), simplifying the test code and making it more consistent with other integration tests in the codebase.

Changes:

  • Removed 18-line my_streq() helper function
  • Added #include "src/string/strcmp.h"
  • Updated string comparisons:
    • Null checks: Direct comparison with nullptr
    • Equality checks: ASSERT_EQ(strcmp(...), 0)
    • Inequality checks: ASSERT_NE(strcmp(...), 0)
  • Added libc.src.string.strcmp dependency in CMakeLists.txt

This reduces the test file from 49 to 31 lines while maintaining the same test coverage. The strcmp-based approach is cleaner and aligns with the pattern used in the environment_management branch tests (setenv_test, unsetenv_test, putenv_test).

Testing: Verified that all test assertions pass with the new implementation.


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

2 Files Affected:

  • (modified) libc/test/integration/src/stdlib/CMakeLists.txt (+1)
  • (modified) libc/test/integration/src/stdlib/getenv_test.cpp (+13-32)
diff --git a/libc/test/integration/src/stdlib/CMakeLists.txt b/libc/test/integration/src/stdlib/CMakeLists.txt
index 1773d9fc9f0f5..caa2a0b96288e 100644
--- a/libc/test/integration/src/stdlib/CMakeLists.txt
+++ b/libc/test/integration/src/stdlib/CMakeLists.txt
@@ -12,6 +12,7 @@ add_integration_test(
     getenv_test.cpp
   DEPENDS
     libc.src.stdlib.getenv
+    libc.src.string.strcmp
   ENV
     FRANCE=Paris
     GERMANY=Berlin
diff --git a/libc/test/integration/src/stdlib/getenv_test.cpp b/libc/test/integration/src/stdlib/getenv_test.cpp
index 72dcea0ddf1f1..b909df94c7fb6 100644
--- a/libc/test/integration/src/stdlib/getenv_test.cpp
+++ b/libc/test/integration/src/stdlib/getenv_test.cpp
@@ -7,43 +7,24 @@
 //===----------------------------------------------------------------------===//
 
 #include "src/stdlib/getenv.h"
+#include "src/string/strcmp.h"
 
 #include "test/IntegrationTest/test.h"
 
-static bool my_streq(const char *lhs, const char *rhs) {
-  if (lhs == rhs)
-    return true;
-  if (((lhs == static_cast<char *>(nullptr)) &&
-       (rhs != static_cast<char *>(nullptr))) ||
-      ((lhs != static_cast<char *>(nullptr)) &&
-       (rhs == static_cast<char *>(nullptr)))) {
-    return false;
-  }
-  const char *l, *r;
-  for (l = lhs, r = rhs; *l != '\0' && *r != '\0'; ++l, ++r)
-    if (*l != *r)
-      return false;
-
-  return *l == '\0' && *r == '\0';
-}
-
 TEST_MAIN([[maybe_unused]] int argc, [[maybe_unused]] char **argv,
           [[maybe_unused]] char **envp) {
-  ASSERT_TRUE(
-      my_streq(LIBC_NAMESPACE::getenv(""), static_cast<char *>(nullptr)));
-  ASSERT_TRUE(
-      my_streq(LIBC_NAMESPACE::getenv("="), static_cast<char *>(nullptr)));
-  ASSERT_TRUE(my_streq(LIBC_NAMESPACE::getenv("MISSING ENV VARIABLE"),
-                       static_cast<char *>(nullptr)));
-  ASSERT_FALSE(
-      my_streq(LIBC_NAMESPACE::getenv("PATH"), static_cast<char *>(nullptr)));
-  ASSERT_TRUE(my_streq(LIBC_NAMESPACE::getenv("FRANCE"), "Paris"));
-  ASSERT_FALSE(my_streq(LIBC_NAMESPACE::getenv("FRANCE"), "Berlin"));
-  ASSERT_TRUE(my_streq(LIBC_NAMESPACE::getenv("GERMANY"), "Berlin"));
-  ASSERT_TRUE(
-      my_streq(LIBC_NAMESPACE::getenv("FRANC"), static_cast<char *>(nullptr)));
-  ASSERT_TRUE(my_streq(LIBC_NAMESPACE::getenv("FRANCE1"),
-                       static_cast<char *>(nullptr)));
+  ASSERT_TRUE(LIBC_NAMESPACE::getenv("") == nullptr);
+  ASSERT_TRUE(LIBC_NAMESPACE::getenv("=") == nullptr);
+  ASSERT_TRUE(LIBC_NAMESPACE::getenv("MISSING ENV VARIABLE") == nullptr);
+  ASSERT_FALSE(LIBC_NAMESPACE::getenv("PATH") == nullptr);
+  ASSERT_EQ(LIBC_NAMESPACE::strcmp(LIBC_NAMESPACE::getenv("FRANCE"), "Paris"),
+            0);
+  ASSERT_NE(LIBC_NAMESPACE::strcmp(LIBC_NAMESPACE::getenv("FRANCE"), "Berlin"),
+            0);
+  ASSERT_EQ(LIBC_NAMESPACE::strcmp(LIBC_NAMESPACE::getenv("GERMANY"), "Berlin"),
+            0);
+  ASSERT_TRUE(LIBC_NAMESPACE::getenv("FRANC") == nullptr);
+  ASSERT_TRUE(LIBC_NAMESPACE::getenv("FRANCE1") == nullptr);
 
   return 0;
 }

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