diff mbox series

[RFCv4,4/4] lib/test_printf.c: add test cases for '%pD'

Message ID 20210615154952.2744-5-justin.he@arm.com (mailing list archive)
State New
Headers show
Series make '%pD' print full path for file | expand

Commit Message

Jia He June 15, 2021, 3:49 p.m. UTC
After the behaviour of specifier '%pD' is changed to print full path
of struct file, the related test cases are also updated.

Given the string of '%pD' is prepended from the end of the buffer, the
check of "wrote beyond the nul-terminator" should be skipped.

Signed-off-by: Jia He <justin.he@arm.com>
---
 lib/test_printf.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

Comments

Andy Shevchenko June 15, 2021, 8:47 p.m. UTC | #1
On Tue, Jun 15, 2021 at 6:55 PM Jia He <justin.he@arm.com> wrote:
>
> After the behaviour of specifier '%pD' is changed to print full path
> of struct file, the related test cases are also updated.
>
> Given the string of '%pD' is prepended from the end of the buffer, the
> check of "wrote beyond the nul-terminator" should be skipped.
>
> Signed-off-by: Jia He <justin.he@arm.com>
> ---
>  lib/test_printf.c | 26 +++++++++++++++++++++++++-
>  1 file changed, 25 insertions(+), 1 deletion(-)
>
> diff --git a/lib/test_printf.c b/lib/test_printf.c
> index d1d2f898ebae..9f851a82b3af 100644
> --- a/lib/test_printf.c
> +++ b/lib/test_printf.c
> @@ -16,6 +16,7 @@
>
>  #include <linux/bitmap.h>
>  #include <linux/dcache.h>
> +#include <linux/fs.h>
>  #include <linux/socket.h>
>  #include <linux/in.h>
>
> @@ -34,6 +35,7 @@ KSTM_MODULE_GLOBALS();
>
>  static char *test_buffer __initdata;
>  static char *alloced_buffer __initdata;
> +static bool is_prepended_buf __initdata;
>
>  extern bool no_hash_pointers;
>
> @@ -78,7 +80,7 @@ do_test(int bufsize, const char *expect, int elen,
>                 return 1;
>         }
>
> -       if (memchr_inv(test_buffer + written + 1, FILL_CHAR, bufsize - (written + 1))) {

> +       if (!is_prepended_buf && memchr_inv(test_buffer + written + 1, FILL_CHAR, bufsize - (written + 1))) {

Can it be parametrized? I don't like the custom test case being
involved here like this.

>                 pr_warn("vsnprintf(buf, %d, \"%s\", ...) wrote beyond the nul-terminator\n",
>                         bufsize, fmt);
>                 return 1;
> @@ -501,6 +503,27 @@ dentry(void)
>         test("  bravo/alfa|  bravo/alfa", "%12pd2|%*pd2", &test_dentry[2], 12, &test_dentry[2]);
>  }
>
> +static struct vfsmount test_vfsmnt __initdata = {};
> +
> +static struct file test_file __initdata = {
> +       .f_path = { .dentry = &test_dentry[2],
> +                   .mnt = &test_vfsmnt,
> +       },
> +};
> +
> +static void __init
> +f_d_path(void)
> +{
> +       test("(null)", "%pD", NULL);
> +       test("(efault)", "%pD", PTR_INVALID);
> +
> +       is_prepended_buf = true;
> +       test("/bravo/alfa   |/bravo/alfa   ", "%-14pD|%*pD", &test_file, -14, &test_file);
> +       test("   /bravo/alfa|   /bravo/alfa", "%14pD|%*pD", &test_file, 14, &test_file);
> +       test("   /bravo/alfa|/bravo/alfa   ", "%14pD|%-14pD", &test_file, &test_file);
> +       is_prepended_buf = false;
> +}
> +
>  static void __init
>  struct_va_format(void)
>  {
> @@ -784,6 +807,7 @@ test_pointer(void)
>         ip();
>         uuid();
>         dentry();
> +       f_d_path();
>         struct_va_format();
>         time_and_date();
>         struct_clk();
> --
> 2.17.1
>
Petr Mladek June 17, 2021, 2:52 p.m. UTC | #2
On Tue 2021-06-15 23:47:29, Andy Shevchenko wrote:
> On Tue, Jun 15, 2021 at 6:55 PM Jia He <justin.he@arm.com> wrote:
> >
> > After the behaviour of specifier '%pD' is changed to print full path
> > of struct file, the related test cases are also updated.
> >
> > Given the string of '%pD' is prepended from the end of the buffer, the
> > check of "wrote beyond the nul-terminator" should be skipped.
> >
> > Signed-off-by: Jia He <justin.he@arm.com>
> > ---
> >  lib/test_printf.c | 26 +++++++++++++++++++++++++-
> >  1 file changed, 25 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/test_printf.c b/lib/test_printf.c
> > index d1d2f898ebae..9f851a82b3af 100644
> > --- a/lib/test_printf.c
> > +++ b/lib/test_printf.c
> > @@ -78,7 +80,7 @@ do_test(int bufsize, const char *expect, int elen,
> >                 return 1;
> >         }
> >
> > -       if (memchr_inv(test_buffer + written + 1, FILL_CHAR, bufsize - (written + 1))) {
> 
> > +       if (!is_prepended_buf && memchr_inv(test_buffer + written + 1, FILL_CHAR, bufsize - (written + 1))) {
> 
> Can it be parametrized? I don't like the custom test case being
> involved here like this.

Yup, it would be nice.

Also it is far from obvious what @is_prepended_buf means if you do not
have context of this patchset. I think about a more generic name
that comes from the wording used in 3rd patch, e.g.

    @need_scratch_space or @using_scratch_space or @dirty_buf

Anyway, the most easy way to pass this as a parameter would be to add it
to __test() and define a wrapper, .e.g:

static void __printf(3, 4) __init
__test(const char *expect, int elen, bool using_scratch_space,
	const char *fmt, ...)

/*
 * More relaxed test for non-standard formats that are using the provided buffer
 * as a scratch space and write beyond the trailing '\0'.
 */
#define test_using_scratch_space(expect, fmt, ...)			\
	__test(expect, strlen(expect), true, fmt, ##__VA_ARGS__)


Best Regards,
Petr
Jia He June 22, 2021, 2:21 a.m. UTC | #3
> -----Original Message-----
> From: Petr Mladek <pmladek@suse.com>
> Sent: Thursday, June 17, 2021 10:53 PM
> To: Andy Shevchenko <andy.shevchenko@gmail.com>
> Cc: Justin He <Justin.He@arm.com>; Steven Rostedt <rostedt@goodmis.org>;
> Sergey Senozhatsky <senozhatsky@chromium.org>; Andy Shevchenko
> <andriy.shevchenko@linux.intel.com>; Rasmus Villemoes
> <linux@rasmusvillemoes.dk>; Jonathan Corbet <corbet@lwn.net>; Alexander
> Viro <viro@zeniv.linux.org.uk>; Linus Torvalds <torvalds@linux-
> foundation.org>; Peter Zijlstra (Intel) <peterz@infradead.org>; Eric
> Biggers <ebiggers@google.com>; Ahmed S. Darwish <a.darwish@linutronix.de>;
> Linux Documentation List <linux-doc@vger.kernel.org>; Linux Kernel Mailing
> List <linux-kernel@vger.kernel.org>; Linux FS Devel <linux-
> fsdevel@vger.kernel.org>; Matthew Wilcox <willy@infradead.org>
> Subject: Re: [PATCH RFCv4 4/4] lib/test_printf.c: add test cases for '%pD'
>
> On Tue 2021-06-15 23:47:29, Andy Shevchenko wrote:
> > On Tue, Jun 15, 2021 at 6:55 PM Jia He <justin.he@arm.com> wrote:
> > >
> > > After the behaviour of specifier '%pD' is changed to print full path
> > > of struct file, the related test cases are also updated.
> > >
> > > Given the string of '%pD' is prepended from the end of the buffer, the
> > > check of "wrote beyond the nul-terminator" should be skipped.
> > >
> > > Signed-off-by: Jia He <justin.he@arm.com>
> > > ---
> > >  lib/test_printf.c | 26 +++++++++++++++++++++++++-
> > >  1 file changed, 25 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/lib/test_printf.c b/lib/test_printf.c
> > > index d1d2f898ebae..9f851a82b3af 100644
> > > --- a/lib/test_printf.c
> > > +++ b/lib/test_printf.c
> > > @@ -78,7 +80,7 @@ do_test(int bufsize, const char *expect, int elen,
> > >                 return 1;
> > >         }
> > >
> > > -       if (memchr_inv(test_buffer + written + 1, FILL_CHAR, bufsize -
> (written + 1))) {
> >
> > > +       if (!is_prepended_buf && memchr_inv(test_buffer + written + 1,
> FILL_CHAR, bufsize - (written + 1))) {
> >
> > Can it be parametrized? I don't like the custom test case being
> > involved here like this.
>
> Yup, it would be nice.
>
> Also it is far from obvious what @is_prepended_buf means if you do not
> have context of this patchset. I think about a more generic name
> that comes from the wording used in 3rd patch, e.g.
>
>     @need_scratch_space or @using_scratch_space or @dirty_buf
>
> Anyway, the most easy way to pass this as a parameter would be to add it
> to __test() and define a wrapper, .e.g:
>
> static void __printf(3, 4) __init
> __test(const char *expect, int elen, bool using_scratch_space,
>       const char *fmt, ...)
>
> /*
>  * More relaxed test for non-standard formats that are using the provided
> buffer
>  * as a scratch space and write beyond the trailing '\0'.
>  */
> #define test_using_scratch_space(expect, fmt, ...)                    \
>       __test(expect, strlen(expect), true, fmt, ##__VA_ARGS__)
Okay, thanks for the suggestion.


--
Cheers,
Justin (Jia He)


IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
diff mbox series

Patch

diff --git a/lib/test_printf.c b/lib/test_printf.c
index d1d2f898ebae..9f851a82b3af 100644
--- a/lib/test_printf.c
+++ b/lib/test_printf.c
@@ -16,6 +16,7 @@ 
 
 #include <linux/bitmap.h>
 #include <linux/dcache.h>
+#include <linux/fs.h>
 #include <linux/socket.h>
 #include <linux/in.h>
 
@@ -34,6 +35,7 @@  KSTM_MODULE_GLOBALS();
 
 static char *test_buffer __initdata;
 static char *alloced_buffer __initdata;
+static bool is_prepended_buf __initdata;
 
 extern bool no_hash_pointers;
 
@@ -78,7 +80,7 @@  do_test(int bufsize, const char *expect, int elen,
 		return 1;
 	}
 
-	if (memchr_inv(test_buffer + written + 1, FILL_CHAR, bufsize - (written + 1))) {
+	if (!is_prepended_buf && memchr_inv(test_buffer + written + 1, FILL_CHAR, bufsize - (written + 1))) {
 		pr_warn("vsnprintf(buf, %d, \"%s\", ...) wrote beyond the nul-terminator\n",
 			bufsize, fmt);
 		return 1;
@@ -501,6 +503,27 @@  dentry(void)
 	test("  bravo/alfa|  bravo/alfa", "%12pd2|%*pd2", &test_dentry[2], 12, &test_dentry[2]);
 }
 
+static struct vfsmount test_vfsmnt __initdata = {};
+
+static struct file test_file __initdata = {
+	.f_path = { .dentry = &test_dentry[2],
+		    .mnt = &test_vfsmnt,
+	},
+};
+
+static void __init
+f_d_path(void)
+{
+	test("(null)", "%pD", NULL);
+	test("(efault)", "%pD", PTR_INVALID);
+
+	is_prepended_buf = true;
+	test("/bravo/alfa   |/bravo/alfa   ", "%-14pD|%*pD", &test_file, -14, &test_file);
+	test("   /bravo/alfa|   /bravo/alfa", "%14pD|%*pD", &test_file, 14, &test_file);
+	test("   /bravo/alfa|/bravo/alfa   ", "%14pD|%-14pD", &test_file, &test_file);
+	is_prepended_buf = false;
+}
+
 static void __init
 struct_va_format(void)
 {
@@ -784,6 +807,7 @@  test_pointer(void)
 	ip();
 	uuid();
 	dentry();
+	f_d_path();
 	struct_va_format();
 	time_and_date();
 	struct_clk();