ip6, escaped_string tests

ip6, escaped_string tests

[Kent Overstreet]

Hey Petr, here's the tests for ipv6 address and string_escape_mem(). I looked at
the clock code and it's just printing the name from a struct lookup - didn't
exactly seem worth a test.

There weren't any regressions from the printbuf patches found by these tests -
however, a recent fixup patch for prt_u64_minwidth() was wrong, I'll send you a
new version of that patch too.

[PATCH 1/2] lib/test_printf.c: Add ip6 tests

[Kent Overstreet]

This adds missing tests for ip6 address: both bare address and
sockaddr_in6, as well as the additional flags for compressed address,
port, scope and flow.

Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
Cc: Petr Mladek <pmladek@suse.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Sergey Senozhatsky <senozhatsky@chromium.org>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 lib/test_printf.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/lib/test_printf.c b/lib/test_printf.c
index 4bd15a593f..6a56dbf076 100644
--- a/lib/test_printf.c
+++ b/lib/test_printf.c
@@ -18,6 +18,7 @@
 #include <linux/dcache.h>
 #include <linux/socket.h>
 #include <linux/in.h>
+#include <linux/in6.h>
 
 #include <linux/gfp.h>
 #include <linux/mm.h>
@@ -61,6 +62,9 @@ do_test(int bufsize, const char *expect, int elen,
 		pr_warn("vsnprintf(buf, %d, \"%s\", ...) returned %d, expected %d\n",
 			bufsize, fmt, ret, elen);
 		return 1;
+		pr_warn("vsnprintf(buf, %d, \"%s\", ...) returned %d, expected %d (%s != %s)\n",
+			bufsize, fmt, ret, elen, test_buffer, expect);
+		return 1;
 	}
 
 	if (memchr_inv(alloced_buffer, FILL_CHAR, PAD_SIZE)) {
@@ -452,6 +456,31 @@ ip4(void)
 static void __init
 ip6(void)
 {
+	struct sockaddr_in6 sa = { .sin6_family = AF_INET6 };
+
+	sa.sin6_port			= cpu_to_be16(12345);
+	sa.sin6_addr.in6_u.u6_addr16[0]	= cpu_to_be16(1);
+	sa.sin6_addr.in6_u.u6_addr16[1]	= cpu_to_be16(2);
+	sa.sin6_addr.in6_u.u6_addr16[6]	= cpu_to_be16(7);
+	sa.sin6_addr.in6_u.u6_addr16[7]	= cpu_to_be16(8);
+	sa.sin6_flowinfo		= cpu_to_be32(8008);
+	sa.sin6_scope_id		= 4004;
+
+	test("00010002000000000000000000070008",		"%pi6", &sa.sin6_addr);
+	test("00010002000000000000000000070008",		"%piS", &sa);
+	test("0001:0002:0000:0000:0000:0000:0007:0008",		"%pI6", &sa.sin6_addr);
+	test("0001:0002:0000:0000:0000:0000:0007:0008",		"%pIS", &sa);
+	test("1:2::7:8",					"%pISc", &sa);
+
+	test("[0001:0002:0000:0000:0000:0000:0007:0008]:12345",	"%pISp", &sa);
+	test("[0001:0002:0000:0000:0000:0000:0007:0008]%4004",	"%pISs", &sa);
+	test("[0001:0002:0000:0000:0000:0000:0007:0008]/8008",	"%pISf", &sa);
+
+	test("[1:2::7:8]:12345",				"%pIScp", &sa);
+	test("[1:2::7:8]%4004",					"%pIScs", &sa);
+	test("[1:2::7:8]/8008",					"%pIScf", &sa);
+
+	test("[1:2::7:8]:12345/8008%4004",			"%pIScpsf", &sa);
 }
 
 static void __init
-- 
2.36.1

[PATCH 2/2] lib/test_printf.c: Add escaped string tests

[Kent Overstreet]

This adds missing tests for %pE, escaped strings.

Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
Cc: Petr Mladek <pmladek@suse.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Sergey Senozhatsky <senozhatsky@chromium.org>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 lib/test_printf.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/lib/test_printf.c b/lib/test_printf.c
index 6a56dbf076..247b1adbbe 100644
--- a/lib/test_printf.c
+++ b/lib/test_printf.c
@@ -413,6 +413,21 @@ addr(void)
 static void __init
 escaped_str(void)
 {
+	const char buf[] = "test \f\n\r\t\v \"\\\a\e \0 end";
+	unsigned n = strlen(buf), with_embedded_nul = sizeof(buf) - 1;
+
+	/* ESCAPE_ANY_NP: */
+	test("test \\f\\n\\r\\t\\v \"\\\\a\\e ",	"%*pE",  n, buf);
+	/* ESCAPE_ANY: */
+	//test("test \\f\\n\\r\\t\\v \"\\\\a\\e end",	"%*pEa",  n, buf);
+	/* ESCAPE_SPACE: */
+	test("test \\f\\n\\r\\t\\v \"\\\x07\x1b ",	"%*pEs", n, buf);
+
+	/* ESCAPE_SPECIAL: */
+	test("test \f\n\r\t\v \\\"\\\\\\a\\e ",		"%*pEc", n, buf);
+
+	/* ESCAPE_NULL: */
+	test("test \f\n\r\t\v \"\\\a\e \\0 end",	"%*pEn", with_embedded_nul, buf);
 }
 
 static void __init
-- 
2.36.1

Re: [PATCH 1/2] lib/test_printf.c: Add ip6 tests

[Sergey Senozhatsky]

On (22/08/29 20:31), Kent Overstreet wrote:
> diff --git a/lib/test_printf.c b/lib/test_printf.c
> index 4bd15a593f..6a56dbf076 100644
> --- a/lib/test_printf.c
> +++ b/lib/test_printf.c
> @@ -18,6 +18,7 @@
>  #include <linux/dcache.h>
>  #include <linux/socket.h>
>  #include <linux/in.h>
> +#include <linux/in6.h>
>  
>  #include <linux/gfp.h>
>  #include <linux/mm.h>
> @@ -61,6 +62,9 @@ do_test(int bufsize, const char *expect, int elen,
>  		pr_warn("vsnprintf(buf, %d, \"%s\", ...) returned %d, expected %d\n",
>  			bufsize, fmt, ret, elen);
>  		return 1;
> +		pr_warn("vsnprintf(buf, %d, \"%s\", ...) returned %d, expected %d (%s != %s)\n",
> +			bufsize, fmt, ret, elen, test_buffer, expect);
> +		return 1;
>  	}

I assume you intended to replace first pr_warn() with the second one?

Re: [PATCH 1/2] lib/test_printf.c: Add ip6 tests

[Rasmus Villemoes]

On 30/08/2022 03.47, Sergey Senozhatsky wrote:
> On (22/08/29 20:31), Kent Overstreet wrote:
>> diff --git a/lib/test_printf.c b/lib/test_printf.c
>> index 4bd15a593f..6a56dbf076 100644
>> --- a/lib/test_printf.c
>> +++ b/lib/test_printf.c
>> @@ -18,6 +18,7 @@
>>  #include <linux/dcache.h>
>>  #include <linux/socket.h>
>>  #include <linux/in.h>
>> +#include <linux/in6.h>
>>  
>>  #include <linux/gfp.h>
>>  #include <linux/mm.h>
>> @@ -61,6 +62,9 @@ do_test(int bufsize, const char *expect, int elen,
>>  		pr_warn("vsnprintf(buf, %d, \"%s\", ...) returned %d, expected %d\n",
>>  			bufsize, fmt, ret, elen);
>>  		return 1;
>> +		pr_warn("vsnprintf(buf, %d, \"%s\", ...) returned %d, expected %d (%s != %s)\n",
>> +			bufsize, fmt, ret, elen, test_buffer, expect);
>> +		return 1;
>>  	}
> 
> I assume you intended to replace first pr_warn() with the second one?

Probably, but that's not ok. The test framework does not trust
vsnprintf(), especially not when it does not behave as expected. So I
very much carefully do not treat the buffer as a nul-terminated string
until I have verified that it does have a nul character (that's tested a
few lines below), and then when I compare the buffer contents can I pass
it as a %s argument. Also note how that test takes the 'we may be
testing a truncated write' into consideration, by printing the expect
string via %.*s.

tl;dr, please just remove that hunk.

Rasmus

Re: [PATCH 2/2] lib/test_printf.c: Add escaped string tests

[Rasmus Villemoes]

On 30/08/2022 02.31, Kent Overstreet wrote:
> This adds missing tests for %pE, escaped strings.
> 
> Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
> Cc: Petr Mladek <pmladek@suse.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Sergey Senozhatsky <senozhatsky@chromium.org>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> ---
>  lib/test_printf.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/lib/test_printf.c b/lib/test_printf.c
> index 6a56dbf076..247b1adbbe 100644
> --- a/lib/test_printf.c
> +++ b/lib/test_printf.c
> @@ -413,6 +413,21 @@ addr(void)
>  static void __init
>  escaped_str(void)
>  {
> +	const char buf[] = "test \f\n\r\t\v \"\\\a\e \0 end";
> +	unsigned n = strlen(buf), with_embedded_nul = sizeof(buf) - 1;
> +
> +	/* ESCAPE_ANY_NP: */
> +	test("test \\f\\n\\r\\t\\v \"\\\\a\\e ",	"%*pE",  n, buf);
> +	/* ESCAPE_ANY: */
> +	//test("test \\f\\n\\r\\t\\v \"\\\\a\\e end",	"%*pEa",  n, buf);

Is there a reason that one is commented out?

> +	/* ESCAPE_SPACE: */
> +	test("test \\f\\n\\r\\t\\v \"\\\x07\x1b ",	"%*pEs", n, buf);
> +
> +	/* ESCAPE_SPECIAL: */
> +	test("test \f\n\r\t\v \\\"\\\\\\a\\e ",		"%*pEc", n, buf);
> +
> +	/* ESCAPE_NULL: */
> +	test("test \f\n\r\t\v \"\\\a\e \\0 end",	"%*pEn", with_embedded_nul, buf);
>  }

Perhaps also include a few byte values >= 128. Otherwise looks good.

Rasmus

Re: [PATCH 1/2] lib/test_printf.c: Add ip6 tests

[Kent Overstreet]

On Tue, Aug 30, 2022 at 09:32:19AM +0200, Rasmus Villemoes wrote:
> On 30/08/2022 03.47, Sergey Senozhatsky wrote:
> > On (22/08/29 20:31), Kent Overstreet wrote:
> >> diff --git a/lib/test_printf.c b/lib/test_printf.c
> >> index 4bd15a593f..6a56dbf076 100644
> >> --- a/lib/test_printf.c
> >> +++ b/lib/test_printf.c
> >> @@ -18,6 +18,7 @@
> >>  #include <linux/dcache.h>
> >>  #include <linux/socket.h>
> >>  #include <linux/in.h>
> >> +#include <linux/in6.h>
> >>  
> >>  #include <linux/gfp.h>
> >>  #include <linux/mm.h>
> >> @@ -61,6 +62,9 @@ do_test(int bufsize, const char *expect, int elen,
> >>  		pr_warn("vsnprintf(buf, %d, \"%s\", ...) returned %d, expected %d\n",
> >>  			bufsize, fmt, ret, elen);
> >>  		return 1;
> >> +		pr_warn("vsnprintf(buf, %d, \"%s\", ...) returned %d, expected %d (%s != %s)\n",
> >> +			bufsize, fmt, ret, elen, test_buffer, expect);
> >> +		return 1;
> >>  	}
> > 
> > I assume you intended to replace first pr_warn() with the second one?
> 
> Probably, but that's not ok. The test framework does not trust
> vsnprintf(), especially not when it does not behave as expected. So I
> very much carefully do not treat the buffer as a nul-terminated string
> until I have verified that it does have a nul character (that's tested a
> few lines below), and then when I compare the buffer contents can I pass
> it as a %s argument. Also note how that test takes the 'we may be
> testing a truncated write' into consideration, by printing the expect
> string via %.*s.
> 
> tl;dr, please just remove that hunk.

Debugging code I meant to remove, whoops.