Along with the parent patch, this should fix the last call to a non internal libc function.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
libc/src/string/strcat.cpp | ||
---|---|---|
17–18 | Remove outdated comment |
libc/src/string/strcat.cpp | ||
---|---|---|
17–18 | Good catch, thanks! |
libc/src/string/strcpy.cpp | ||
---|---|---|
16 | We could make these __restrict while we're here but I think it there might be no benefit given we just call memcpy. | |
libc/src/string/strlen.cpp | ||
17 | Brackets not common for single statement loops. | |
libc/src/string/strlen.h | ||
1 | We could add the -*- C++ -*- | |
libc/test/src/string/strlen_test.cpp | ||
17 | Maybe 0UL and 12UL whichever you think looks better. |
Couple of nits inline but LGTM.
libc/src/string/strlen.cpp | ||
---|---|---|
15–21 | Can you add a TODO here saying that one could improve this by making use of the alignment and target machine word size information? | |
libc/src/string/strlen.h | ||
1 | This problem is probably more wide spread than just the string directory. +1 to fixing them all up in a separate, may be obvious patch. | |
13 | Do you still need to include stddef.h if you include include/string.h? In my patches, I have included the public header iff required. For example, the public header defines a header which is required by the API declared here. So, keeping stddef.h and removing include/string.h is also fine. Whatever you choose, share your thoughts so that I can learn if I am missing something. |
libc/src/string/strlen.h | ||
---|---|---|
13 | I mean't to say, "For example, if the public header defines a *struct* which is required by the API declared here." |
libc/src/string/strlen.cpp | ||
---|---|---|
15–21 | Added a more general comment about looking into the performance of this function. | |
libc/src/string/strlen.h | ||
13 | Thanks for bringing this up! I based this off the memcpy header without much consideration. Now that I think about it more, it seems that both strlen and memcpy indirectly include stddef.h from include/string.h. I think it makes more sense to use the include/string.h approach as that will be more consistent and would require less code duplication in terms of figuring out which entrypoints depend on which headers. |
libc/src/string/strlen.h | ||
---|---|---|
1 | Agreed I can send something out for this shortly. |
libc/src/string/strlen.cpp | ||
---|---|---|
18 | This is great suggestion thank you! (: |
Remove outdated comment