[libc-commits] [PATCH] D147346: [libc] Add strchrnul implementation

[Caslyn Tonelli via Phabricator via libc-commits]

Caslyn added inline comments.


================
Comment at: libc/spec/gnu_ext.td:79
+            RetValSpec<CharPtr>,
+            [ArgSpec<ConstCharPtr>, ArgSpec<ConstCharPtr>]
+        >,
----------------
michaelrj wrote:
> this second ArgSpec should be `IntType`
thanks for catching!


================
Comment at: libc/src/string/strchrnul.cpp:18
+LLVM_LIBC_FUNCTION(char *, strchrnul, (const char *src, int c)) {
+  char *ch = __llvm_libc::strchr(src, c);
+  return ch ? ch : const_cast<char *>(src) + internal::string_length(src);
----------------
michaelrj wrote:
> In general, we avoid calling other entrypoints from with our functions to keep the library more modular. Having strchrnul include strchr would mean that to include strchrnul in a final build it would also need to include strchr. It's unlikely to be a problem here, but it is a problem for functions like atoi and strtol.
> 
> In this case the strchr function is short enough that it can probably just be copied in here.
Gotcha - will remember that. Copied in the `strchr` logic in the updated patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147346