mbox series

[v3,0/4] Misc fixes for strlcpy() and strlcat()

Message ID 20240218195110.1386840-1-rodrigo@sdfg.com.ar
Headers
Series Misc fixes for strlcpy() and strlcat() |

Message

Rodrigo Campos Feb. 18, 2024, 7:51 p.m. UTC 
  As requested by Willy and Thomas[1], here go some more fixes and tests for
strlcpy() and strlcat().

The first patch just fixes the compilation when the compiler might replace some
code with its strlen() implementation, which will not be found. Therefore, we
just export it as that can happen also on user-code, outside of nolibc.

The rest of the commits:
	* Fix the return code of both functions
	* Make sure to always null-terminate the dst buffer
	* Honor the size parameter as documented
	* Add tests for both functions

All has been checked against the corresponding libbsd implementation[2].

Let me know what you think 🙂

---
Changes from v2:
 * Add v3 to the subject, previously I wasn't using v<revision>
 * Make strlcat() and strlcpy() have a shorter size when compiled
 * Make src and dst buffer sizes different in test and add trailing chars, so we
   can easily detect more bugs.

[1]: https://lore.kernel.org/all/9538a8fe-b92f-42a5-99d1-25969cf51647@sdfg.com.ar/
[2]: https://gitlab.freedesktop.org/libbsd/libbsd.git


Rodrigo Campos (4):
  tools/nolibc/string: export strlen()
  tools/nolibc: Fix strlcat() return code and size usage
  tools/nolibc: Fix strlcpy() return code and size usage
  selftests/nolibc: Add tests for strlcat() and strlcpy()

 tools/include/nolibc/string.h                | 47 ++++++++++++--------
 tools/testing/selftests/nolibc/nolibc-test.c | 40 +++++++++++++++++
 2 files changed, 69 insertions(+), 18 deletions(-)

Comments

Willy Tarreau Feb. 19, 2024, 8:48 p.m. UTC | #1 
Hi Rodrigo,

On Sun, Feb 18, 2024 at 04:51:02PM -0300, Rodrigo Campos wrote:
> As requested by Willy and Thomas[1], here go some more fixes and tests for
> strlcpy() and strlcat().
> 
> The first patch just fixes the compilation when the compiler might replace some
> code with its strlen() implementation, which will not be found. Therefore, we
> just export it as that can happen also on user-code, outside of nolibc.
> 
> The rest of the commits:
> 	* Fix the return code of both functions
> 	* Make sure to always null-terminate the dst buffer
> 	* Honor the size parameter as documented
> 	* Add tests for both functions
> 
> All has been checked against the corresponding libbsd implementation[2].
> 
> Let me know what you think ?

This time everything looked good to me and I queued them into the fixes
branch since they address a real corner-case bug. I finally decided not
to change your comment for '/*' on a single line because it turns out
that the file in question almost exclusively uses the shorter, net-style
comments like you did, and you were probably inspired by the surrounding
ones.

Many thanks for your work and your patience ;-)
Willy
Rodrigo Campos Feb. 20, 2024, 7:25 p.m. UTC | #2 
On 2/19/24 17:48, Willy Tarreau wrote:
> Hi Rodrigo,
>  > I finally decided not
> to change your comment for '/*' on a single line because it turns out
> that the file in question almost exclusively uses the shorter, net-style
> comments like you did, and you were probably inspired by the surrounding
> ones.

Heh, makes sense, I usually do check the file style before writing the 
first comment. Maybe it was that, hard to know :D

> 
> Many thanks for your work and your patience ;-)

Thank you!