[libc-commits] [PATCH] D85779: [libc] Add strtok_r implementation.

Chris Gyurgyik via Phabricator via libc-commits libc-commits at lists.llvm.org
Wed Aug 12 18:15:37 PDT 2020

cgyurgyik marked 5 inline comments as done.
cgyurgyik added inline comments.

Comment at: libc/src/string/strtok_r.cpp:16
+char *LLVM_LIBC_ENTRYPOINT(strtok_r)(char *src, const char *delimiter_string,
+                                     char **saveptr) {
cgyurgyik wrote:
> abrachet wrote:
> > cgyurgyik wrote:
> > > cgyurgyik wrote:
> > > > cgyurgyik wrote:
> > > > > abrachet wrote:
> > > > > > sivachandra wrote:
> > > > > > > Would it make sense to implement `strtok` and `strtok_r` over a common function which is equivalent in functionality to `strtok_r`?
> > > > > > Making the prototype for this function `restrict` but not the function itself doesn't allow for optimizations. `src` and `delimiter_string` should be marked `__restrict` here and I imagine in a couple of other string functions too
> > > > > Yeah you're right, I haven't been using `__restrict` at all. I should've probably asked about this at an earlier point, wasn't sure if there was a reason to use it or not. Will fix this here, and other places in future revisions as well.
> > > > I'm struggling to get a static inline function working for both a static char * and a char ** without running into lifetime issues with the `saveptr`. Was there something you had in mind? Maybe I'm missing something simple here.
> > > One way I could do this is to have the utility function return a struct `{char *token, char *strtok_string}`, and then set the `strtok_string` to the respective value, `static char *` for strtok, and `dereferenced char**` for strtok_r. Then we can return the `struct.token`.
> > `strtok` can just be
> > ```lang=cpp
> > char *LLVM_LIBC_ENTRYPOINT(strtok)(char *src, const char *delimiter_string) {
> >   static char *strtok_str;
> >   return __llvm_libc::strtok_r(src, delimiter_string, &strtok_str);
> > }
> > ```
> > There is no lifetime issue.
> Yep... that's the simple solution I avoided for much too long. I was trying to do the reverse. Will fix. Thanks abrachet :)
Left a TODO for myself to address this in a separate revision, since this applies to other functions as well.

  rG LLVM Github Monorepo



More information about the libc-commits mailing list