[libc][stdlib] Simplify getenv_test by using strcmp instead of custom helper

[kaladron]

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.

[llvmbot]

@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; } 

[michaelrj-google]

LGTM, thanks for the cleanup

[lntue]

Can you use inline_strcmp from https://github.com/llvm/llvm-project/blob/main/libc/src/string/memory_utils/inline_strcmp.h instead, so that this test won't depend on libc.src.string.strcmp entrypoint?

[kaladron]

Yes, I'll make this update. I was away the past 2 weeks and am still unburying myself but will get to this and the other one shortly. Sorry for the delay!

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.