This is an archive of the discontinued LLVM Phabricator instance.

[libc] Add strlcat
ClosedPublic

Authored by abrachet on May 19 2022, 8:08 AM.

Diff Detail

Event Timeline

abrachet created this revision.May 19 2022, 8:08 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 19 2022, 8:08 AM
abrachet requested review of this revision.May 19 2022, 8:08 AM
michaelrj added inline comments.May 19 2022, 11:39 AM
libc/src/string/strlcat.cpp
20–21

this doesn't quite work because the return value always needs to be strlen(dst)+strlen(src). This means you always need to find the full length of dst.

libc/test/src/string/strlcat_test.cpp
34

as discussed above the return value of strlcat should always be strlen(dst)+strlen(src), so this should be 4.

abrachet marked 2 inline comments as done.May 19 2022, 12:07 PM
abrachet added inline comments.
libc/src/string/strlcat.cpp
20–21

Described below

libc/test/src/string/strlcat_test.cpp
34

No because the buffer size was passed as 1, so strlcat only reads 1 char from buf and uses that as the size.

Per the man page "Note, however, that if strlcat() traverses size characters without finding a NUL, the length of the string is considered to be size and the destination string will not be NUL-terminated (since there was no space for the NUL)."

michaelrj accepted this revision.May 19 2022, 12:47 PM

LGTM

libc/test/src/string/strlcat_test.cpp
34

oops, you're right. The source I was going off of was wrong, and this way matches the existing implementation.

This revision is now accepted and ready to land.May 19 2022, 12:47 PM
michaelrj added inline comments.May 19 2022, 12:49 PM
libc/src/string/strlcat.cpp
24

nit: it looks like len is only used here, so you could move the call to string_length inside this if to save time on the other branch.

abrachet updated this revision to Diff 430812.May 19 2022, 2:46 PM
abrachet marked 4 inline comments as done.
This revision was landed with ongoing or failed builds.May 19 2022, 2:48 PM
Closed by commit rGb1183305f882: [libc] Add strlcat (authored by abrachet). · Explain Why
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMay 19 2022, 2:48 PM

Just pointing out that the review thread is not getting copied to libc-commits.