This is an archive of the discontinued LLVM Phabricator instance.

[libc] Add strchrnul implementation
ClosedPublic

Authored by Caslyn on Mar 31 2023, 11:33 AM.

Details

Summary

Introduce strchrnul implementation and unit tests.

Ticket: https://fxbug.dev/124219

Diff Detail

Event Timeline

Caslyn created this revision.Mar 31 2023, 11:33 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptMar 31 2023, 11:33 AM
Caslyn requested review of this revision.Mar 31 2023, 11:33 AM
Caslyn updated this revision to Diff 510089.Mar 31 2023, 11:39 AM

Remove extraneous line

Mostly LGTM with minor changes.

libc/spec/gnu_ext.td
79

this second ArgSpec should be IntType

libc/src/string/strchrnul.cpp
18

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.

Caslyn updated this revision to Diff 510131.Mar 31 2023, 2:28 PM
Caslyn marked an inline comment as done.
  • Corrected IntType parameter
  • Removed strchr dep, copied in logic
libc/spec/gnu_ext.td
79

thanks for catching!

libc/src/string/strchrnul.cpp
18

Gotcha - will remember that. Copied in the strchr logic in the updated patch.

michaelrj accepted this revision.Mar 31 2023, 3:59 PM

LGTM after addressing the last comment.

libc/src/string/strchrnul.cpp
21–22

I think this can be simplified to just return ch.

This revision is now accepted and ready to land.Mar 31 2023, 3:59 PM
Caslyn updated this revision to Diff 510154.Mar 31 2023, 5:18 PM
  • Simplified return statement
libc/src/string/strchrnul.cpp
21–22

Done (assumed you mean src).

This revision was automatically updated to reflect the committed changes.