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.
This adds missing tests for %pE, escaped strings.
Signed-off-by: Kent Overstreet <[email protected]>
Cc: Petr Mladek <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Sergey Senozhatsky <[email protected]>
Cc: Andy Shevchenko <[email protected]>
Cc: Rasmus Villemoes <[email protected]>
---
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
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 <[email protected]>
Cc: Petr Mladek <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Sergey Senozhatsky <[email protected]>
Cc: Andy Shevchenko <[email protected]>
Cc: Rasmus Villemoes <[email protected]>
---
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
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?
On 30/08/2022 02.31, Kent Overstreet wrote:
> This adds missing tests for %pE, escaped strings.
>
> Signed-off-by: Kent Overstreet <[email protected]>
> Cc: Petr Mladek <[email protected]>
> Cc: Steven Rostedt <[email protected]>
> Cc: Sergey Senozhatsky <[email protected]>
> Cc: Andy Shevchenko <[email protected]>
> Cc: Rasmus Villemoes <[email protected]>
> ---
> 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
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
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.