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

[Siva Chandra via Phabricator via libc-commits]

sivachandra accepted this revision.
sivachandra marked an inline comment as done.
sivachandra added a comment.
This revision is now accepted and ready to land.

Couple of nits inline but LGTM.



================
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;
----------------
Can you add a TODO here saying that one could improve this by making use of the alignment and target machine word size information?


================
Comment at: libc/src/string/strlen.h:1
-//===-------------------- Implementation of strcpy -----------------------===//
+//===----------------- Implementation header for strlen -------------------===//
 //
----------------
PaulkaToast wrote:
> abrachet wrote:
> > We could add the `-*- C++ -*-`
> Just noticed none of the headers in string have this.  I'll go make a separate patch for that. Thanks!
This problem is probably more wide spread than just the `string` directory. +1 to fixing them all up in a separate, may be obvious patch.


================
Comment at: libc/src/string/strlen.h:13
+#include "include/string.h"
+#include <stddef.h> // size_t
 
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77279