[libc-commits] [PATCH] D77279: [libc] Add strlen implementation.

[Paula Toth via Phabricator via libc-commits]

PaulkaToast added inline comments.


================
Comment at: libc/src/string/strlen.cpp:15
 
-char *LLVM_LIBC_ENTRYPOINT(strcpy)(char *dest, const char *src) {
-  return reinterpret_cast<char *>(
-      __llvm_libc::memcpy(dest, src, ::strlen(src) + 1));
+size_t LLVM_LIBC_ENTRYPOINT(strlen)(const char *src) {
+  const char *end = src;
----------------
sivachandra wrote:
> Can you add a TODO here saying that one could improve this by making use of the alignment and target machine word size information?
Added a more general comment about looking into the performance of this function.


================
Comment at: libc/src/string/strlen.h:13
+#include "include/string.h"
+#include <stddef.h> // size_t
 
----------------
sivachandra wrote:
> sivachandra wrote:
> > Do you still need to include `stddef.h` if you include `include/string.h`? In my patches, I have included the public header iff required. For example, the public header defines a header which is required by the API declared here. So, keeping `stddef.h` and removing `include/string.h` is also fine. Whatever you choose, share your thoughts so that I can learn if I am missing something.
> I mean't to say, "For example, if the public header defines a *struct* which is required by the API declared here."
Thanks for bringing this up! I based this off the memcpy header without much consideration. 

Now that I think about it more, it seems that both strlen and memcpy indirectly include stddef.h from include/string.h. I think it makes more sense to use  the include/string.h approach as that will be more consistent and would require less code duplication in terms of figuring out which entrypoints depend on which headers. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77279