AZero13 · AZero13 · Jul 25, 2021 · Jul 23, 2021 · Jul 23, 2021 · Jul 23, 2021
diff --git a/libc/src/string/memchr.cpp b/libc/src/string/memchr.cpp
@@ -17,7 +17,8 @@ namespace __llvm_libc {
 // TODO: Look at performance benefits of comparing words.
 LLVM_LIBC_FUNCTION(void *, memchr, (const void *src, int c, size_t n)) {
   return internal::find_first_character(
-      reinterpret_cast<const unsigned char *>(src), c, n);
+      reinterpret_cast<const unsigned char *>(src),
+      static_cast<unsigned char>(c), n);
 }
 
 } // namespace __llvm_libc
diff --git a/libc/src/string/memmove.cpp b/libc/src/string/memmove.cpp
@@ -15,22 +15,25 @@
 
 namespace __llvm_libc {
 
-static inline void move_byte_forward(char *dest_m, const char *src_m,
-                                     size_t count) {
-  for (size_t offset = 0; count; --count, ++offset)
+static inline void move_byte_forward(unsigned char *dest_m,
+                                     const unsigned char *src_m, size_t count) {
+  for (size_t offset = 0; offset != count; ++offset)
     dest_m[offset] = src_m[offset];
 }
 
-static inline void move_byte_backward(char *dest_m, const char *src_m,
+static inline void move_byte_backward(unsigned char *dest_m,
+                                      const unsigned char *src_m,
                                       size_t count) {
-  for (size_t offset = count - 1; count; --count, --offset)
-    dest_m[offset] = src_m[offset];
+  if (count != 0) { // Avoid underflow
+    for (size_t offset = count - 1; offset != 0; --offset)
+      dest_m[offset] = src_m[offset];
+  }
 }
 
 LLVM_LIBC_FUNCTION(void *, memmove,
                    (void *dest, const void *src, size_t count)) {
-  char *dest_c = reinterpret_cast<char *>(dest);
-  const char *src_c = reinterpret_cast<const char *>(src);
+  unsigned char *dest_c = reinterpret_cast<unsigned char *>(dest);
+  const unsigned char *src_c = reinterpret_cast<const unsigned char *>(src);
 
   // If the distance between src_c and dest_c is equal to or greater
   // than count (integerAbs(src_c - dest_c) >= count), they would not overlap.
@@ -58,7 +61,7 @@ LLVM_LIBC_FUNCTION(void *, memmove,
   // TODO: Optimize `move_byte_xxx(...)` functions.
   if (dest_c < src_c)
     move_byte_forward(dest_c, src_c, count);
-  if (dest_c > src_c)
+  else if (dest_c > src_c)
     move_byte_backward(dest_c, src_c, count);
   return dest;
 }

diff --git a/libc/src/string/memory_utils/elements.h b/libc/src/string/memory_utils/elements.h
@@ -96,9 +96,8 @@ template <typename Element, size_t ElementCount> struct Repeated {
     for (size_t i = 0; i < ElementCount; ++i) {
       const size_t offset = i * Element::kSize;
       // We make the assumption that 'Equals' si cheaper than 'ThreeWayCompare'.
-      if (Element::Equals(lhs + offset, rhs + offset))
-        continue;
-      return Element::ThreeWayCompare(lhs + offset, rhs + offset);
+      if (!Element::Equals(lhs + offset, rhs + offset))
+        return Element::ThreeWayCompare(lhs + offset, rhs + offset);
     }
     return 0;
   }
@@ -131,10 +130,10 @@ template <typename Head, typename... Tail> struct Chained<Head, Tail...> {
   }
 
   static int ThreeWayCompare(const char *lhs, const char *rhs) {
-    if (__llvm_libc::Equals<Head>(lhs, rhs))
-      return Chained<Tail...>::ThreeWayCompare(lhs + Head::kSize,
-                                               rhs + Head::kSize);
-    return __llvm_libc::ThreeWayCompare<Head>(lhs, rhs);
+    if (!__llvm_libc::Equals<Head>(lhs, rhs))
+      return __llvm_libc::ThreeWayCompare<Head>(lhs, rhs);
+    return Chained<Tail...>::ThreeWayCompare(lhs + Head::kSize,
+                                             rhs + Head::kSize);
   }
 
   static void SplatSet(char *dst, const unsigned char value) {

diff --git a/libc/src/string/memrchr.cpp b/libc/src/string/memrchr.cpp
@@ -14,11 +14,10 @@ namespace __llvm_libc {
 
 LLVM_LIBC_FUNCTION(void *, memrchr, (const void *src, int c, size_t n)) {
   const unsigned char *str = reinterpret_cast<const unsigned char *>(src);
-  const unsigned char ch = c;
+  const unsigned char ch = static_cast<unsigned char>(c);
   for (; n != 0; --n) {
-    const unsigned char *s = str + n - 1;
-    if (*s == ch)
-      return const_cast<unsigned char *>(s);
+    if (*(--str) == ch)
+      return const_cast<unsigned char *>(str);
   }
   return nullptr;
 }

diff --git a/libc/src/string/strchr.cpp b/libc/src/string/strchr.cpp
@@ -14,12 +14,12 @@ namespace __llvm_libc {
 
 // TODO: Look at performance benefits of comparing words.
 LLVM_LIBC_FUNCTION(char *, strchr, (const char *src, int c)) {
-  unsigned char *str =
-      const_cast<unsigned char *>(reinterpret_cast<const unsigned char *>(src));
-  const unsigned char ch = c;
-  for (; *str && *str != ch; ++str)
-    ;
-  return *str == ch ? reinterpret_cast<char *>(str) : nullptr;
+  const char ch = static_cast<char>(c);
+  for (; *src != ch; ++src)
+    if (*src == '\0')
+      return nullptr;
+
+  return const_cast<char *>(src);
 }
 
 } // namespace __llvm_libc
diff --git a/libc/src/string/string_utils.h b/libc/src/string/string_utils.h
@@ -18,30 +18,31 @@ namespace internal {
 // Returns the length of a string, denoted by the first occurrence
 // of a null terminator.
 static inline size_t string_length(const char *src) {
-  size_t length;
-  for (length = 0; *src; ++src, ++length)
+  const char *const initial = src;
+  for (; *src != '\0'; ++src)
     ;
-  return length;
+  return src - initial;
 }
 
 // Returns the first occurrence of 'ch' within the first 'n' characters of
 // 'src'. If 'ch' is not found, returns nullptr.
 static inline void *find_first_character(const unsigned char *src,
                                          unsigned char ch, size_t n) {
-  for (; n && *src != ch; --n, ++src)
-    ;
-  return n ? const_cast<unsigned char *>(src) : nullptr;
+  for (; n != 0; --n, ++src)
+    if (*src == ch)
+      return const_cast<unsigned char *>(src);
+  return nullptr;
 }
 
 // Returns the maximum length span that contains only characters not found in
 // 'segment'. If no characters are found, returns the length of 'src'.
 static inline size_t complementary_span(const char *src, const char *segment) {
-  const char *initial = src;
+  const char *const initial = src;
   cpp::Bitset<256> bitset;
 
-  for (; *segment; ++segment)
+  for (; *segment != '\0'; ++segment)
     bitset.set(*segment);
-  for (; *src && !bitset.test(*src); ++src)
+  for (; *src != '\0' && !bitset.test(*src); ++src)
     ;
   return src - initial;
 }
@@ -59,22 +60,21 @@ static inline char *string_token(char *__restrict src,
                                  const char *__restrict delimiter_string,
                                  char **__restrict saveptr) {
   cpp::Bitset<256> delimiter_set;
-  for (; *delimiter_string; ++delimiter_string)
+  for (; *delimiter_string != '\0'; ++delimiter_string)
     delimiter_set.set(*delimiter_string);
 
   src = src ? src : *saveptr;
-  for (; *src && delimiter_set.test(*src); ++src)
+  for (; *src != '\0' && delimiter_set.test(*src); ++src)
     ;
-  if (!*src) {
+  if (*src == '\0') {
     *saveptr = src;
     return nullptr;
   }
   char *token = src;
-  for (; *src && !delimiter_set.test(*src); ++src)
+  for (; *src != '\0' && !delimiter_set.test(*src); ++src)
     ;
-  if (*src) {
-    *src = '\0';
-    ++src;
+  if (*src != '\0') {
+    *src++ = '\0';
   }
   *saveptr = src;
   return token;

diff --git a/libc/src/string/strrchr.cpp b/libc/src/string/strrchr.cpp
@@ -13,12 +13,12 @@
 namespace __llvm_libc {
 
 LLVM_LIBC_FUNCTION(char *, strrchr, (const char *src, int c)) {
-  const char ch = c;
+  const char ch = static_cast<char>(c);
   char *last_occurrence = nullptr;
   do {
     if (*src == ch)
       last_occurrence = const_cast<char *>(src);
-  } while (*src++);
+  } while (*src++ != '\0');
   return last_occurrence;
 }