This is an archive of the discontinued LLVM Phabricator instance.

[libc] add strdup implementation
ClosedPublic

Authored by michaelrj on Oct 11 2021, 4:28 PM.

Details

Summary

Add an implementation for strdup.

Diff Detail

Event Timeline

michaelrj created this revision.Oct 11 2021, 4:28 PM
michaelrj requested review of this revision.Oct 11 2021, 4:28 PM
michaelrj updated this revision to Diff 379188.Oct 12 2021, 2:26 PM

update and add a new test

michaelrj updated this revision to Diff 379190.Oct 12 2021, 2:29 PM

small fix

libc/include/stdlib.h.def
17–22 ↗(On Diff #379188)

This feels wrong, but I'm not sure how else to get these into the header

michaelrj edited the summary of this revision. (Show Details)Oct 12 2021, 2:30 PM
michaelrj added a reviewer: lntue.
sivachandra added inline comments.
libc/include/stdlib.h.def
17–22 ↗(On Diff #379188)

This is a good find. Assuming that allocators are but one of the "external" functions that LLVM libc will provide, I think we should have a scalable solution for this. So, may be add a new cmake rule called add_external_entrypoint which does nothing but create a dummy target that can be listed in places like entrypoints.txt?

libc/src/string/strdup.cpp
25

This is a C library implementation. So, the style should be snake_case for variable names. Or, you can completely avoid this conundrum by renaming the src to old and newStr to str.

lntue added inline comments.Oct 18 2021, 8:17 AM
libc/include/stdlib.h.def
17–22 ↗(On Diff #379188)

+1. I wasn't able to find a better way, and was waiting for Siva for a better suggestion.

michaelrj updated this revision to Diff 380781.Oct 19 2021, 2:57 PM
michaelrj marked 3 inline comments as done.

add a new path for external functions to be included in the appropriate header file.

Can we separate the CMake and allocator changes into a different patch? Rest of the patch is OK but will accept after we separate.

michaelrj updated this revision to Diff 380804.Oct 19 2021, 4:11 PM

split the cmake changes into their own patch

michaelrj updated this revision to Diff 380809.Oct 19 2021, 4:14 PM

missed the changes in stdc.td

michaelrj edited the summary of this revision. (Show Details)Oct 19 2021, 4:15 PM
michaelrj updated this revision to Diff 382116.Oct 25 2021, 2:03 PM

fix cmake rules to match the new way external entrypoints are handled

sivachandra accepted this revision.Oct 25 2021, 9:38 PM
This revision is now accepted and ready to land.Oct 25 2021, 9:38 PM
lntue accepted this revision.Oct 26 2021, 7:43 AM
lntue added inline comments.
libc/test/src/string/CMakeLists.txt
100

nit: alignment.

michaelrj marked an inline comment as done.

fix indent

This revision was landed with ongoing or failed builds.Oct 27 2021, 10:21 AM
This revision was automatically updated to reflect the committed changes.