[libc-commits] [PATCH] D82134: Add strcmp.

[Siva Chandra via Phabricator via libc-commits]

sivachandra added inline comments.


================
Comment at: libc/src/string/strcmp.cpp:15
+
+int LLVM_LIBC_ENTRYPOINT(strcmp)(const char *l, const char *r) {
+  while (*l) {
----------------
Is there benefit in comparing words at a time? I know that it leads to potential OOB reads, so we can handle it separately. But if it can lead to speed up, just a leave a TODO about it and we will get to it in a later round.


================
Comment at: libc/src/string/strcmp.cpp:22
+  }
+  return *(const unsigned char *)l - *(const unsigned char *)r;
+}
----------------
Use reinterpret cast instead of C style casts.


================
Comment at: libc/test/src/string/strcmp_test.cpp:11
+#include "utils/UnitTest/Test.h"
+
+TEST(StrCmpTest, EmptyStringsShouldReturnZero) {
----------------
Where relevant in the following tests, can you add a check with the operands reversed?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82134/new/

https://reviews.llvm.org/D82134