[libc-commits] [PATCH] D82724: [libc] Add memchr implementation.

[Chris Gyurgyik via Phabricator via libc-commits]

cgyurgyik marked 7 inline comments as done.
cgyurgyik added inline comments.


================
Comment at: libc/src/string/memchr.cpp:18
+  const unsigned char *str = reinterpret_cast<const unsigned char *>(src);
+  const unsigned char ch = static_cast<unsigned char>(c);
+  for (; n && *str != ch; --n, ++str)
----------------
sivachandra wrote:
> Do you need the explicit static cast? Without it, wouldn't the compile time implicit cast do the same thing?
Semantically they're the same thing. I just assumed that when narrowing or widening, its always better to be explicit. 


================
Comment at: libc/test/src/string/memchr_test.cpp:32
+  ASSERT_EQ(ret[0], 'b');
+  ASSERT_EQ(ret[1], 'c');
+}
----------------
sivachandra wrote:
> Ditto.
Since src is not null-terminated, ASSERT_STREQ is going to look beyond the end of the array. I could add a null terminator to the end of ret, but don't know if that's much better than this approach. Is there an approach you were thinking of?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82724