Mailing List Archive

[PATCH v3 4/4] selftests/nolibc: Add tests for strlcat() and strlcpy()
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() [ In reply to ]
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() [ In reply to ]
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() [ In reply to ]
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 4/4] selftests/nolibc: Add tests for strlcat() and strlcpy() [ In reply to ]
+ 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() [ In reply to ]
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() [ In reply to ]
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() [ In reply to ]
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