[libc-commits] [PATCH] D147822: [libc] Add memmem implementation

[Caslyn Tonelli via Phabricator via libc-commits]

Caslyn added a comment.

Thanks for the quick response @gchatelet!



================
Comment at: libc/src/string/memory_utils/memmem_implementations.h:18
+constexpr static void *
+memmem_implementation(const void *haystack, size_t haystack_len,
+                      const void *needle, size_t needle_len, Comp &&comp) {
----------------
gchatelet wrote:
> Not necessarily for this patch but it may be better to use the `cpp::string_view` abstraction for both needle and haystack.
> 
> We should also return a `const char*` to keep types as sane as possible.
> This way we `const_cast` only where it actually matters, i.e. in `memmem`.
> 
I'll be sure to address this in a follow up patch. 


================
Comment at: libc/test/src/string/memmem_test.cpp:97
+    char h[] = {'\0'};
+    char n[] = {
+        '\0',
----------------
mcgrathr wrote:
> gchatelet wrote:
> > same here
> Note that clang-format will use separate lines for each element when there is a trailing comma before the end-brace, and will compact things when there is no trailing comma.
> 
Ah - thanks. I noticed this behavior with clang-format but wasn't sure how to work around it. Done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147822