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

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

[Rodrigo Campos]

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(-)

-- 
2.43.0

[PATCH v3 1/4] tools/nolibc/string: export strlen()

[Rodrigo Campos]

As with commit 8d304a374023, "tools/nolibc/string: export memset() and
memmove()", gcc -Os without -ffreestanding may fail to compile with:

	cc -fno-asynchronous-unwind-tables -fno-ident -s -Os -nostdlib -lgcc  -static -o test test.c
	/usr/bin/ld: /tmp/cccIasKL.o: in function `main':
	test.c:(.text.startup+0x1e): undefined reference to `strlen'
	collect2: error: ld returned 1 exit status

As on the aforementioned commit, this patch adds a section to export
this function so compilation works on those cases too.

Signed-off-by: Rodrigo Campos <rodrigo@sdfg.com.ar>
---
 tools/include/nolibc/string.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/include/nolibc/string.h b/tools/include/nolibc/string.h
index a01c69dd495f..ed15c22b1b2a 100644
--- a/tools/include/nolibc/string.h
+++ b/tools/include/nolibc/string.h
@@ -123,7 +123,7 @@ char *strcpy(char *dst, const char *src)
  * thus itself, hence the asm() statement below that's meant to disable this
  * confusing practice.
  */
-static __attribute__((unused))
+__attribute__((weak,unused,section(".text.nolibc_strlen")))
 size_t strlen(const char *str)
 {
 	size_t len;
-- 
2.43.0

[PATCH v3 2/4] tools/nolibc: Fix strlcat() return code and size usage

[Rodrigo Campos]

The return code should always be strlen(src) + strnlen(dst, size).

Let's make sure to copy at most size-1 bytes from src and null-terminate
the dst buffer if we did copied something.

While we can use strnlen() and strncpy() to implement strlcat(), this is
simple enough and results in shorter code when compiled.

Signed-off-by: Rodrigo Campos <rodrigo@sdfg.com.ar>
---
 tools/include/nolibc/string.h | 27 ++++++++++++++++++---------
 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/tools/include/nolibc/string.h b/tools/include/nolibc/string.h
index ed15c22b1b2a..cc51fd6b63d0 100644
--- a/tools/include/nolibc/string.h
+++ b/tools/include/nolibc/string.h
@@ -187,22 +187,31 @@ char *strndup(const char *str, size_t maxlen)
 static __attribute__((unused))
 size_t strlcat(char *dst, const char *src, size_t size)
 {
-	size_t len;
-	char c;
+	size_t len = 0;
 
-	for (len = 0; dst[len];	len++)
-		;
+	for (; len < size; len++) {
+		if (dst[len] == '\0')
+			break;
+	}
 
-	for (;;) {
-		c = *src;
-		if (len < size)
-			dst[len] = c;
-		if (!c)
+	/*
+	 * We want len < size-1. But as size is unsigned and can wrap
+	 * around, we use len + 1 instead.
+	 */
+	while (len + 1 < size) {
+		dst[len] = *src;
+		if (*src == '\0')
 			break;
 		len++;
 		src++;
 	}
 
+	if (len < size)
+		dst[len] = '\0';
+
+	while (*src++)
+		len++;
+
 	return len;
 }
 
-- 
2.43.0

[PATCH v3 3/4] tools/nolibc: Fix strlcpy() return code and size usage

[Rodrigo Campos]

The return code should always be strlen(src), and we should copy at most
size-1 bytes.

While we are there, make sure to null-terminate the dst buffer if we
copied something.

Signed-off-by: Rodrigo Campos <rodrigo@sdfg.com.ar>
---
 tools/include/nolibc/string.h | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/tools/include/nolibc/string.h b/tools/include/nolibc/string.h
index cc51fd6b63d0..565230a4ad47 100644
--- a/tools/include/nolibc/string.h
+++ b/tools/include/nolibc/string.h
@@ -219,16 +219,18 @@ static __attribute__((unused))
 size_t strlcpy(char *dst, const char *src, size_t size)
 {
 	size_t len;
-	char c;
 
-	for (len = 0;;) {
-		c = src[len];
-		if (len < size)
-			dst[len] = c;
-		if (!c)
-			break;
-		len++;
+	for (len = 0; len < size; len++) {
+		dst[len] = src[len];
+		if (!dst[len])
+			return len;
 	}
+	if (size)
+		dst[size-1] = '\0';
+
+	while (src[len])
+		len++;
+
 	return len;
 }
 
-- 
2.43.0

[PATCH v3 4/4] selftests/nolibc: Add tests for strlcat() and strlcpy()

[Rodrigo Campos]

I've verified that the tests matches libbsd's strlcat()/strlcpy()
implementation.

Please note that as strlcat()/strlcpy() are not part of the libc, the
tests are only compiled when using nolibc.

Signed-off-by: Rodrigo Campos <rodrigo@sdfg.com.ar>
---
 tools/testing/selftests/nolibc/nolibc-test.c | 40 ++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
index 6ba4f8275ac4..d373fc14706c 100644
--- a/tools/testing/selftests/nolibc/nolibc-test.c
+++ b/tools/testing/selftests/nolibc/nolibc-test.c
@@ -600,6 +600,25 @@ int expect_strne(const char *expr, int llen, const char *cmp)
 	return ret;
 }
 
+#define EXPECT_STRBUFEQ(cond, expr, buf, val, cmp)				\
+	do { if (!(cond)) result(llen, SKIPPED); else ret += expect_str_buf_eq(expr, buf, val, llen, cmp); } while (0)
+
+static __attribute__((unused))
+int expect_str_buf_eq(size_t expr, const char *buf, size_t val, int llen, const char *cmp)
+{
+	llen += printf(" = %lu <%s> ", expr, buf);
+	if (strcmp(buf, cmp) != 0) {
+		result(llen, FAIL);
+		return 1;
+	}
+	if (expr != val) {
+		result(llen, FAIL);
+		return 1;
+	}
+
+	result(llen, OK);
+	return 0;
+}
 
 /* declare tests based on line numbers. There must be exactly one test per line. */
 #define CASE_TEST(name) \
@@ -991,6 +1010,14 @@ int run_stdlib(int min, int max)
 	for (test = min; test >= 0 && test <= max; test++) {
 		int llen = 0; /* line length */
 
+		/* For functions that take a long buffer, like strlcat()
+		 * Add some more chars after the \0, to test functions that overwrite the buffer set
+		 * the \0 at the exact right position.
+		 */
+		char buf[10] = "test123456";
+		buf[4] = '\0';
+
+
 		/* avoid leaving empty lines below, this will insert holes into
 		 * test numbers.
 		 */
@@ -1007,6 +1034,19 @@ int run_stdlib(int min, int max)
 		CASE_TEST(strchr_foobar_z);    EXPECT_STRZR(1, strchr("foobar", 'z')); break;
 		CASE_TEST(strrchr_foobar_o);   EXPECT_STREQ(1, strrchr("foobar", 'o'), "obar"); break;
 		CASE_TEST(strrchr_foobar_z);   EXPECT_STRZR(1, strrchr("foobar", 'z')); break;
+#ifdef NOLIBC
+		CASE_TEST(strlcat_0);          EXPECT_STRBUFEQ(1, strlcat(buf, "bar", 0), buf, 3, "test"); break;
+		CASE_TEST(strlcat_1);          EXPECT_STRBUFEQ(1, strlcat(buf, "bar", 1), buf, 4, "test"); break;
+		CASE_TEST(strlcat_5);          EXPECT_STRBUFEQ(1, strlcat(buf, "bar", 5), buf, 7, "test"); break;
+		CASE_TEST(strlcat_6);          EXPECT_STRBUFEQ(1, strlcat(buf, "bar", 6), buf, 7, "testb"); break;
+		CASE_TEST(strlcat_7);          EXPECT_STRBUFEQ(1, strlcat(buf, "bar", 7), buf, 7, "testba"); break;
+		CASE_TEST(strlcat_8);          EXPECT_STRBUFEQ(1, strlcat(buf, "bar", 8), buf, 7, "testbar"); break;
+		CASE_TEST(strlcpy_0);          EXPECT_STRBUFEQ(1, strlcpy(buf, "bar", 0), buf, 3, "test"); break;
+		CASE_TEST(strlcpy_1);          EXPECT_STRBUFEQ(1, strlcpy(buf, "bar", 1), buf, 3, ""); break;
+		CASE_TEST(strlcpy_2);          EXPECT_STRBUFEQ(1, strlcpy(buf, "bar", 2), buf, 3, "b"); break;
+		CASE_TEST(strlcpy_3);          EXPECT_STRBUFEQ(1, strlcpy(buf, "bar", 3), buf, 3, "ba"); break;
+		CASE_TEST(strlcpy_4);          EXPECT_STRBUFEQ(1, strlcpy(buf, "bar", 4), buf, 3, "bar"); break;
+#endif
 		CASE_TEST(memcmp_20_20);       EXPECT_EQ(1, memcmp("aaa\x20", "aaa\x20", 4), 0); break;
 		CASE_TEST(memcmp_20_60);       EXPECT_LT(1, memcmp("aaa\x20", "aaa\x60", 4), 0); break;
 		CASE_TEST(memcmp_60_20);       EXPECT_GT(1, memcmp("aaa\x60", "aaa\x20", 4), 0); break;
-- 
2.43.0

Re: [PATCH v3 4/4] selftests/nolibc: Add tests for strlcat() and strlcpy()

[Rodrigo Campos]

On 2/18/24 16:51, Rodrigo Campos wrote:
> diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
> index 6ba4f8275ac4..d373fc14706c 100644
> --- a/tools/testing/selftests/nolibc/nolibc-test.c
> +++ b/tools/testing/selftests/nolibc/nolibc-test.c
> @@ -600,6 +600,25 @@ int expect_strne(const char *expr, int llen, const char *cmp)
>   /* declare tests based on line numbers. There must be exactly one test per line. */
>   #define CASE_TEST(name) \
> @@ -991,6 +1010,14 @@ int run_stdlib(int min, int max)
>   	for (test = min; test >= 0 && test <= max; test++) {
>   		int llen = 0; /* line length */
>   
> +		/* For functions that take a long buffer, like strlcat()

Ouch, I didn't leave a "/*\n" for this comment :(

Re: [PATCH v3 4/4] selftests/nolibc: Add tests for strlcat() and strlcpy()

[Willy Tarreau]

On Sun, Feb 18, 2024 at 05:39:03PM -0300, Rodrigo Campos wrote:
> On 2/18/24 16:51, Rodrigo Campos wrote:
> > diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
> > index 6ba4f8275ac4..d373fc14706c 100644
> > --- a/tools/testing/selftests/nolibc/nolibc-test.c
> > +++ b/tools/testing/selftests/nolibc/nolibc-test.c
> > @@ -600,6 +600,25 @@ int expect_strne(const char *expr, int llen, const char *cmp)
> >   /* declare tests based on line numbers. There must be exactly one test per line. */
> >   #define CASE_TEST(name) \
> > @@ -991,6 +1010,14 @@ int run_stdlib(int min, int max)
> >   	for (test = min; test >= 0 && test <= max; test++) {
> >   		int llen = 0; /* line length */
> > +		/* For functions that take a long buffer, like strlcat()
> 
> Ouch, I didn't leave a "/*\n" for this comment :(

No worries, if you want I'll address it myself, just let me know, no
need to respin a series for this.

Thanks!
Willy

Re: [PATCH v3 4/4] selftests/nolibc: Add tests for strlcat() and strlcpy()

[Rodrigo Campos]

On 2/18/24 18:11, Willy Tarreau wrote:
> On Sun, Feb 18, 2024 at 05:39:03PM -0300, Rodrigo Campos wrote:
>> On 2/18/24 16:51, Rodrigo Campos wrote:
>>> diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
>>> index 6ba4f8275ac4..d373fc14706c 100644
>>> --- a/tools/testing/selftests/nolibc/nolibc-test.c
>>> +++ b/tools/testing/selftests/nolibc/nolibc-test.c
>>> @@ -600,6 +600,25 @@ int expect_strne(const char *expr, int llen, const char *cmp)
>>>    /* declare tests based on line numbers. There must be exactly one test per line. */
>>>    #define CASE_TEST(name) \
>>> @@ -991,6 +1010,14 @@ int run_stdlib(int min, int max)
>>>    	for (test = min; test >= 0 && test <= max; test++) {
>>>    		int llen = 0; /* line length */
>>> +		/* For functions that take a long buffer, like strlcat()
>>
>> Ouch, I didn't leave a "/*\n" for this comment :(
> 
> No worries, if you want I'll address it myself, just let me know, no
> need to respin a series for this.

Please do, thanks! :)



Best,
Rodrigo

Re: [PATCH v3 0/4] Misc fixes for strlcpy() and strlcat()

[Willy Tarreau]

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

Re: [PATCH v3 0/4] Misc fixes for strlcpy() and strlcat()

[Rodrigo Campos]

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!

Re: [PATCH v3 4/4] selftests/nolibc: Add tests for strlcat() and strlcpy()

[Thomas Weißschuh]

+ Shuah and Paul, please see below.

So I picked this up and it got picked up by Shuah, but...

On 2024-02-18 16:51:06+0000, Rodrigo Campos wrote:
> I've verified that the tests matches libbsd's strlcat()/strlcpy()
> implementation.
> 
> Please note that as strlcat()/strlcpy() are not part of the libc, the
> tests are only compiled when using nolibc.
> 
> Signed-off-by: Rodrigo Campos <rodrigo@sdfg.com.ar>
> ---
>  tools/testing/selftests/nolibc/nolibc-test.c | 40 ++++++++++++++++++++
>  1 file changed, 40 insertions(+)
> 
> diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
> index 6ba4f8275ac4..d373fc14706c 100644
> --- a/tools/testing/selftests/nolibc/nolibc-test.c
> +++ b/tools/testing/selftests/nolibc/nolibc-test.c
> @@ -600,6 +600,25 @@ int expect_strne(const char *expr, int llen, const char *cmp)
>  	return ret;
>  }
>  
> +#define EXPECT_STRBUFEQ(cond, expr, buf, val, cmp)				\
> +	do { if (!(cond)) result(llen, SKIPPED); else ret += expect_str_buf_eq(expr, buf, val, llen, cmp); } while (0)
> +
> +static __attribute__((unused))
> +int expect_str_buf_eq(size_t expr, const char *buf, size_t val, int llen, const char *cmp)
> +{
> +	llen += printf(" = %lu <%s> ", expr, buf);

This introduces a compiler warning on 32bit:

    i386-linux-gcc -Os -fno-ident -fno-asynchronous-unwind-tables -std=c89 -W -Wall -Wextra -fno-stack-protector -m32 -mstack-protector-guard=global -fstack-protector-all  -o nolibc-test \
      -nostdlib -nostdinc -static -Isysroot/i386/include nolibc-test.c nolibc-test-linkage.c -lgcc
    nolibc-test.c: In function 'expect_str_buf_eq':
    nolibc-test.c:610:30: error: format '%lu' expects argument of type 'long unsigned int', but argument 2 has type 'size_t' {aka 'unsigned int'} [-Werror=format=]
      610 |         llen += printf(" = %lu <%s> ", expr, buf);
          |                            ~~^         ~~~~
          |                              |         |
          |                              |         size_t {aka unsigned int}
          |                              long unsigned int
          |                            %u


It is easy enough to fix through a cast to "unsigned long".

The original patch was already sent to Shuah and included in -next.

Shuah, Paul:

I'd like to rewrite the offending commit instead of having a fixup commit.
As it seems to me the original patch would only go into 6.10 anyways.

Any objections?

Notes to self:

* Add flag to run-tests.sh to use -Werror
* Implement "%zu" in nolibc printf()

> +	if (strcmp(buf, cmp) != 0) {
> +		result(llen, FAIL);
> +		return 1;
> +	}
> +	if (expr != val) {
> +		result(llen, FAIL);
> +		return 1;
> +	}
> +
> +	result(llen, OK);
> +	return 0;
> +}

> [..]

Re: [PATCH v3 4/4] selftests/nolibc: Add tests for strlcat() and strlcpy()

[Willy Tarreau]

Hi Thomas,

On Tue, Apr 23, 2024 at 11:18:06AM +0200, Thomas Weißschuh wrote:
> + Shuah and Paul, please see below.
> 
> So I picked this up and it got picked up by Shuah, but...
> 
> On 2024-02-18 16:51:06+0000, Rodrigo Campos wrote:
> > I've verified that the tests matches libbsd's strlcat()/strlcpy()
> > implementation.
> > 
> > Please note that as strlcat()/strlcpy() are not part of the libc, the
> > tests are only compiled when using nolibc.
> > 
> > Signed-off-by: Rodrigo Campos <rodrigo@sdfg.com.ar>
> > ---
> >  tools/testing/selftests/nolibc/nolibc-test.c | 40 ++++++++++++++++++++
> >  1 file changed, 40 insertions(+)
> > 
> > diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
> > index 6ba4f8275ac4..d373fc14706c 100644
> > --- a/tools/testing/selftests/nolibc/nolibc-test.c
> > +++ b/tools/testing/selftests/nolibc/nolibc-test.c
> > @@ -600,6 +600,25 @@ int expect_strne(const char *expr, int llen, const char *cmp)
> >  	return ret;
> >  }
> >  
> > +#define EXPECT_STRBUFEQ(cond, expr, buf, val, cmp)				\
> > +	do { if (!(cond)) result(llen, SKIPPED); else ret += expect_str_buf_eq(expr, buf, val, llen, cmp); } while (0)
> > +
> > +static __attribute__((unused))
> > +int expect_str_buf_eq(size_t expr, const char *buf, size_t val, int llen, const char *cmp)
> > +{
> > +	llen += printf(" = %lu <%s> ", expr, buf);
> 
> This introduces a compiler warning on 32bit:
> 
>     i386-linux-gcc -Os -fno-ident -fno-asynchronous-unwind-tables -std=c89 -W -Wall -Wextra -fno-stack-protector -m32 -mstack-protector-guard=global -fstack-protector-all  -o nolibc-test \
>       -nostdlib -nostdinc -static -Isysroot/i386/include nolibc-test.c nolibc-test-linkage.c -lgcc
>     nolibc-test.c: In function 'expect_str_buf_eq':
>     nolibc-test.c:610:30: error: format '%lu' expects argument of type 'long unsigned int', but argument 2 has type 'size_t' {aka 'unsigned int'} [-Werror=format=]
>       610 |         llen += printf(" = %lu <%s> ", expr, buf);
>           |                            ~~^         ~~~~
>           |                              |         |
>           |                              |         size_t {aka unsigned int}
>           |                              long unsigned int
>           |                            %u
> 
> 
> It is easy enough to fix through a cast to "unsigned long".

Yes, that's usually what I do as well with size_t everywhere as well.

> The original patch was already sent to Shuah and included in -next.
> 
> Shuah, Paul:
> 
> I'd like to rewrite the offending commit instead of having a fixup commit.
> As it seems to me the original patch would only go into 6.10 anyways.
> 
> Any objections?

I, too, think it would be the cleanest history-wise, I just don't know
if that's the easiest for Shuah if it requires to change already published
commit IDs.

> Notes to self:
> 
> * Add flag to run-tests.sh to use -Werror
> * Implement "%zu" in nolibc printf()

Could be nice indeed!

Thanks Thomas for taking care of this!
Willy

Re: [PATCH v3 4/4] selftests/nolibc: Add tests for strlcat() and strlcpy()

[Paul E. McKenney]

On Tue, Apr 23, 2024 at 06:28:31PM +0200, Willy Tarreau wrote:
> Hi Thomas,
> 
> On Tue, Apr 23, 2024 at 11:18:06AM +0200, Thomas Weißschuh wrote:
> > + Shuah and Paul, please see below.
> > 
> > So I picked this up and it got picked up by Shuah, but...
> > 
> > On 2024-02-18 16:51:06+0000, Rodrigo Campos wrote:
> > > I've verified that the tests matches libbsd's strlcat()/strlcpy()
> > > implementation.
> > > 
> > > Please note that as strlcat()/strlcpy() are not part of the libc, the
> > > tests are only compiled when using nolibc.
> > > 
> > > Signed-off-by: Rodrigo Campos <rodrigo@sdfg.com.ar>
> > > ---
> > >  tools/testing/selftests/nolibc/nolibc-test.c | 40 ++++++++++++++++++++
> > >  1 file changed, 40 insertions(+)
> > > 
> > > diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
> > > index 6ba4f8275ac4..d373fc14706c 100644
> > > --- a/tools/testing/selftests/nolibc/nolibc-test.c
> > > +++ b/tools/testing/selftests/nolibc/nolibc-test.c
> > > @@ -600,6 +600,25 @@ int expect_strne(const char *expr, int llen, const char *cmp)
> > >  	return ret;
> > >  }
> > >  
> > > +#define EXPECT_STRBUFEQ(cond, expr, buf, val, cmp)				\
> > > +	do { if (!(cond)) result(llen, SKIPPED); else ret += expect_str_buf_eq(expr, buf, val, llen, cmp); } while (0)
> > > +
> > > +static __attribute__((unused))
> > > +int expect_str_buf_eq(size_t expr, const char *buf, size_t val, int llen, const char *cmp)
> > > +{
> > > +	llen += printf(" = %lu <%s> ", expr, buf);
> > 
> > This introduces a compiler warning on 32bit:
> > 
> >     i386-linux-gcc -Os -fno-ident -fno-asynchronous-unwind-tables -std=c89 -W -Wall -Wextra -fno-stack-protector -m32 -mstack-protector-guard=global -fstack-protector-all  -o nolibc-test \
> >       -nostdlib -nostdinc -static -Isysroot/i386/include nolibc-test.c nolibc-test-linkage.c -lgcc
> >     nolibc-test.c: In function 'expect_str_buf_eq':
> >     nolibc-test.c:610:30: error: format '%lu' expects argument of type 'long unsigned int', but argument 2 has type 'size_t' {aka 'unsigned int'} [-Werror=format=]
> >       610 |         llen += printf(" = %lu <%s> ", expr, buf);
> >           |                            ~~^         ~~~~
> >           |                              |         |
> >           |                              |         size_t {aka unsigned int}
> >           |                              long unsigned int
> >           |                            %u
> > 
> > 
> > It is easy enough to fix through a cast to "unsigned long".
> 
> Yes, that's usually what I do as well with size_t everywhere as well.
> 
> > The original patch was already sent to Shuah and included in -next.
> > 
> > Shuah, Paul:
> > 
> > I'd like to rewrite the offending commit instead of having a fixup commit.
> > As it seems to me the original patch would only go into 6.10 anyways.
> > 
> > Any objections?
> 
> I, too, think it would be the cleanest history-wise, I just don't know
> if that's the easiest for Shuah if it requires to change already published
> commit IDs.

One way would be to resend the series and have Shuah replace what she
has with the new series.  But I don't know enough about Shuah's process
to judge.

							Thanx, Paul

> > Notes to self:
> > 
> > * Add flag to run-tests.sh to use -Werror
> > * Implement "%zu" in nolibc printf()
> 
> Could be nice indeed!
> 
> Thanks Thomas for taking care of this!
> Willy

Re: [PATCH v3 4/4] selftests/nolibc: Add tests for strlcat() and strlcpy()

[Rodrigo Campos]

On 4/23/24 10:18 AM, Thomas Weißschuh wrote:
>> +#define EXPECT_STRBUFEQ(cond, expr, buf, val, cmp)				\
>> +	do { if (!(cond)) result(llen, SKIPPED); else ret += expect_str_buf_eq(expr, buf, val, llen, cmp); } while (0)
>> +
>> +static __attribute__((unused))
>> +int expect_str_buf_eq(size_t expr, const char *buf, size_t val, int llen, const char *cmp)
>> +{
>> +	llen += printf(" = %lu <%s> ", expr, buf);
> 
> This introduces a compiler warning on 32bit:
> 
>      i386-linux-gcc -Os -fno-ident -fno-asynchronous-unwind-tables -std=c89 -W -Wall -Wextra -fno-stack-protector -m32 -mstack-protector-guard=global -fstack-protector-all  -o nolibc-test \
>        -nostdlib -nostdinc -static -Isysroot/i386/include nolibc-test.c nolibc-test-linkage.c -lgcc
>      nolibc-test.c: In function 'expect_str_buf_eq':
>      nolibc-test.c:610:30: error: format '%lu' expects argument of type 'long unsigned int', but argument 2 has type 'size_t' {aka 'unsigned int'} [-Werror=format=]
>        610 |         llen += printf(" = %lu <%s> ", expr, buf);
>            |                            ~~^         ~~~~
>            |                              |         |
>            |                              |         size_t {aka unsigned int}
>            |                              long unsigned int
>            |                            %u
> 
> 
> It is easy enough to fix through a cast to "unsigned long".

Thanks for fixing it! Of course, don't hesitate to let me know if there 
is something I can help with :)



Best,
Rodrigo