[libc-commits] [PATCH] D111584: [libc] add strdup implementation

[Siva Chandra via Phabricator via libc-commits]

sivachandra added inline comments.


================
Comment at: libc/include/stdlib.h.def:17-22
+__BEGIN_C_DECLS
+  void* malloc(size_t);
+  void* calloc(size_t, size_t);
+  void* realloc(void*, size_t);
+  void free(void*);
+__END_C_DECLS
----------------
michaelrj wrote:
> This feels wrong, but I'm not sure how else to get these into the header
This is a good find. Assuming that allocators are but one of the "external" functions that LLVM libc will provide, I think we should have a scalable solution for this. So, may be add a new cmake rule called `add_external_entrypoint` which does nothing but create a dummy target that can be listed in places like `entrypoints.txt`?


================
Comment at: libc/src/string/strdup.cpp:24
+  size_t len = internal::string_length(src) + 1;
+  char *newStr = reinterpret_cast<char *>(::malloc(len)); // NOLINT
+  if (newStr == nullptr) {
----------------
This is a C library implementation. So, the style should be `snake_case` for variable names. Or, you can completely avoid this conundrum by renaming the `src` to `old` and `newStr` to `str`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111584