Skip to content

Conversation

@nitinseshadri
Copy link
Contributor

This PR adds an implementation and tests for strlcpy, and corrects its signature in string.h. strlcpy is a BSD variant of strncpy that ensures the destination is null-terminated. More info about strlcpy here: https://linux.die.net/man/3/strlcpy

@ZERICO2005
Copy link
Contributor

One good thing to check for is that a call to strlcpy is emitted in lto.src. Sometimes Clang will replace it with its own implementation.

@nitinseshadri
Copy link
Contributor Author

nitinseshadri commented Mar 23, 2025

Yep, I checked lto.src and there are calls to strlcpy where I expected them. I also specified -O0 in the Makefile for the test program.

@calc84maniac
Copy link
Contributor

I was going to do a review, but as was mentioned in chat it would be good to unconditionally do the strlen at the start and avoid doing it twice. My other feedback for now is that sbc hl,hl is always more optimized than ld hl,0, even if you have to explicitly clear the carry flag first.

nitinseshadri and others added 2 commits March 23, 2025 19:00
* Use sbc hl, hl rather than ld hl, 0 (carry was already cleared with xor a,a)
* Unconditionally do strlen at the start to avoid doing it twice
@calc84maniac
Copy link
Contributor

Did my own optimization pass which saves 14 bytes if I counted right, let me know if anything doesn't make sense or I somehow broke something. Felt that would be easier than trying to point out individual changes in a review

@nitinseshadri
Copy link
Contributor Author

Thanks, it makes sense to me, and the test program works properly too.

@mateoconlechuga
Copy link
Member

I'm not sure how I feel adding BSD-specific implementations. I can understand some POSIX functions, but an argument needs to be made before we decide to start pulling in random BSD stuff which clutters things and makes it hard for users to know what is actually available. I would prefer to only see C-standard compliant libc.

@nitinseshadri
Copy link
Contributor Author

nitinseshadri commented Mar 25, 2025

strlcpy and strlcat — along with memmem — were added to string.h in the latest POSIX standard, POSIX.1-2024: https://pubs.opengroup.org/onlinepubs/9799919799/basedefs/string.h.html (referred to as "Issue 8" on this page)

@ZERICO2005
Copy link
Contributor

ZERICO2005 commented Mar 25, 2025

quite a few of these POSIX functions are already available as Clang builtins too https://github.com/llvm/llvm-project/blob/release/15.x/clang/include/clang/Basic/Builtins.def

@mateoconlechuga mateoconlechuga merged commit 9b6ee44 into CE-Programming:master Mar 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants