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

[Michael Jones via Phabricator via libc-commits]

michaelrj added a comment.

Mostly LGTM with minor changes.



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


================
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);
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147346