[libc] fix behavior of strrchr(x, '\0')

`strrchr("foo", '\0')` is defined to point to the end of `foo`, rather
than returning NULL. This wasn't caught by tests, since llvm-libc's
`ASSERT_STREQ(nullptr, "");` is not an assertion error.

While I'm here, refactor the test slightly to check for NULL more
specifically. I considered adding fancier `ASSERT`s (and changing the
semantics of `ASSERT_STREQ`), but opted for a more local fix by fair
dice roll.

Author: gburgessiv

libc/src/string/string_utils.h

@@ -239,11 +239,13 @@ LIBC_INLINE constexpr static char *strrchr_implementation(const char *src,
                                                           int c) {
   char ch = static_cast<char>(c);
   char *last_occurrence = nullptr;
-  for (; *src; ++src) {
+  while (true) {
     if (*src == ch)
       last_occurrence = const_cast<char *>(src);
+    if (!*src)
+      return last_occurrence;
+    ++src;
   }
-  return last_occurrence;
 }
 
 } // namespace internal

libc/test/src/string/StrchrTest.h

@@ -40,14 +40,16 @@ template <auto Func> struct StrchrTest : public LIBC_NAMESPACE::testing::Test {
     const char *src = "abcde";
 
     // Should return null terminator.
-    ASSERT_STREQ(Func(src, '\0'), "");
+    const char *nul_terminator = Func(src, '\0');
+    ASSERT_TRUE(nul_terminator != nullptr);
+    ASSERT_STREQ(nul_terminator, "");
     // Source string should not change.
     ASSERT_STREQ(src, "abcde");
   }
 
   void characterNotWithinStringShouldReturnNullptr() {
     // Since 'z' is not within the string, should return nullptr.
-    ASSERT_STREQ(Func("123?", 'z'), nullptr);
+    ASSERT_TRUE(Func("123?", 'z') == nullptr);
   }
 
   void theSourceShouldNotChange() {
@@ -74,11 +76,12 @@ template <auto Func> struct StrchrTest : public LIBC_NAMESPACE::testing::Test {
 
   void emptyStringShouldOnlyMatchNullTerminator() {
     // Null terminator should match.
-    ASSERT_STREQ(Func("", '\0'), "");
+    const char empty_string[] = "";
+    ASSERT_TRUE(Func(empty_string, '\0') == empty_string);
     // All other characters should not match.
-    ASSERT_STREQ(Func("", 'Z'), nullptr);
-    ASSERT_STREQ(Func("", '3'), nullptr);
-    ASSERT_STREQ(Func("", '*'), nullptr);
+    ASSERT_TRUE(Func("", 'Z') == nullptr);
+    ASSERT_TRUE(Func("", '3') == nullptr);
+    ASSERT_TRUE(Func("", '*') == nullptr);
   }
 };
 
@@ -114,25 +117,25 @@ template <auto Func> struct StrrchrTest : public LIBC_NAMESPACE::testing::Test {
     const char *src = "abcde";
 
     // Should return null terminator.
-    ASSERT_STREQ(Func(src, '\0'), "");
+    ASSERT_TRUE(Func(src, '\0') != nullptr);
     // Source string should not change.
     ASSERT_STREQ(src, "abcde");
   }
 
   void findsLastBehindFirstNullTerminator() {
     static const char src[6] = {'a', 'a', '\0', 'b', '\0', 'c'};
     // 'b' is behind a null terminator, so should not be found.
-    ASSERT_STREQ(Func(src, 'b'), nullptr);
+    ASSERT_TRUE(Func(src, 'b') == nullptr);
     // Same goes for 'c'.
-    ASSERT_STREQ(Func(src, 'c'), nullptr);
+    ASSERT_TRUE(Func(src, 'c') == nullptr);
 
     // Should find the second of the two a's.
     ASSERT_STREQ(Func(src, 'a'), "a");
   }
 
   void characterNotWithinStringShouldReturnNullptr() {
     // Since 'z' is not within the string, should return nullptr.
-    ASSERT_STREQ(Func("123?", 'z'), nullptr);
+    ASSERT_TRUE(Func("123?", 'z') == nullptr);
   }
 
   void shouldFindLastOfDuplicates() {
@@ -146,11 +149,12 @@ template <auto Func> struct StrrchrTest : public LIBC_NAMESPACE::testing::Test {
 
   void emptyStringShouldOnlyMatchNullTerminator() {
     // Null terminator should match.
-    ASSERT_STREQ(Func("", '\0'), "");
+    const char empty_string[] = "";
+    ASSERT_TRUE(Func(empty_string, '\0') == empty_string);
     // All other characters should not match.
-    ASSERT_STREQ(Func("", 'A'), nullptr);
-    ASSERT_STREQ(Func("", '2'), nullptr);
-    ASSERT_STREQ(Func("", '*'), nullptr);
+    ASSERT_TRUE(Func("", 'A') == nullptr);
+    ASSERT_TRUE(Func("", '2') == nullptr);
+    ASSERT_TRUE(Func("", '*') == nullptr);
   }
 };