All of lore.kernel.org
 help / color / mirror / Atom feed
From: Brendan Higgins <brendanhiggins@google.com>
To: frowand.list@gmail.com, gregkh@linuxfoundation.org,
	jpoimboe@redhat.com,  keescook@google.com,
	kieran.bingham@ideasonboard.com, mcgrof@kernel.org,
	 peterz@infradead.org, robh@kernel.org, sboyd@kernel.org,
	shuah@kernel.org,  tytso@mit.edu, yamada.masahiro@socionext.com
Cc: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>,
	pmladek@suse.com, linux-doc@vger.kernel.org, amir73il@gmail.com,
	Brendan Higgins <brendanhiggins@google.com>,
	dri-devel@lists.freedesktop.org, Alexander.Levin@microsoft.com,
	linux-kselftest@vger.kernel.org,
	Stephen Rothwell <sfr@canb.auug.org.au>,
	linux-nvdimm@lists.01.org, khilman@baylibre.com,
	knut.omang@oracle.com, wfg@linux.intel.com, joel@jms.id.au,
	rientjes@google.com, jdike@addtoit.com, dan.carpenter@oracle.com,
	devicetree@vger.kernel.org, linux-kbuild@vger.kernel.org,
	Tim.Bird@sony.com, linux-um@lists.infradead.org,
	rostedt@goodmis.org, julia.lawall@lip6.fr,
	kunit-dev@googlegroups.com, richard@nod.at,
	torvalds@linux-foundation.org, rdunlap@infradead.org,
	linux-kernel@vger.kernel.org, daniel@ffwll.ch,
	mpe@ellerman.id.au, linux-fsdevel@vger.kernel.org,
	Joe Perches <joe@perches.com>
Subject: [PATCH v16 19/19] kunit: fix failure to build without printk
Date: Fri, 20 Sep 2019 16:19:23 -0700	[thread overview]
Message-ID: <20190920231923.141900-20-brendanhiggins@google.com> (raw)
In-Reply-To: <20190920231923.141900-1-brendanhiggins@google.com>

Previously KUnit assumed that printk would always be present, which is
not a valid assumption to make. Fix that by removing call to
vprintk_emit, and calling printk directly.

This fixes a build error[1] reported by Randy.

For context this change comes after much discussion. My first stab[2] at
this was just to make the KUnit logging code compile out; however, it
was agreed that if we were going to use vprintk_emit, then vprintk_emit
should provide a no-op stub, which lead to my second attempt[3]. In
response to me trying to stub out vprintk_emit, Sergey Senozhatsky
suggested a way for me to remove our usage of vprintk_emit, which led to
my third attempt at solving this[4].

In my third version of this patch[4], I completely removed vprintk_emit,
as suggested by Sergey; however, there was a bit of debate over whether
Sergey's solution was the best. The debate arose due to Sergey's version
resulting in a checkpatch warning, which resulted in a debate over
correct printk usage. Joe Perches offered an alternative fix which was
somewhat less far reaching than what Sergey had suggested and
importantly relied on continuing to use %pV. Much of the debated
centered around whether %pV should be widely used, and whether Sergey's
version would result in object size bloat. Ultimately, we decided to go
with Sergey's version.

Reported-by: Randy Dunlap <rdunlap@infradead.org>
Link[1]: https://lore.kernel.org/linux-kselftest/c7229254-0d90-d90e-f3df-5b6d6fc0b51f@infradead.org/
Link[2]: https://lore.kernel.org/linux-kselftest/20190827174932.44177-1-brendanhiggins@google.com/
Link[3]: https://lore.kernel.org/linux-kselftest/20190827234835.234473-1-brendanhiggins@google.com/
Link[4]: https://lore.kernel.org/linux-kselftest/20190828093143.163302-1-brendanhiggins@google.com/
Cc: Stephen Rothwell <sfr@canb.auug.org.au>
Cc: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Cc: Joe Perches <joe@perches.com>
Cc: Tim.Bird@sony.com
Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
Acked-by: Randy Dunlap <rdunlap@infradead.org> # build-tested
Reviewed-by: Petr Mladek <pmladek@suse.com>
---
 include/kunit/test.h |  5 ++--
 lib/kunit/test.c     | 57 +++++---------------------------------------
 2 files changed, 8 insertions(+), 54 deletions(-)

diff --git a/include/kunit/test.h b/include/kunit/test.h
index 8b7eb03d4971..dba48304b3bd 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -339,9 +339,8 @@ static inline void *kunit_kzalloc(struct kunit *test, size_t size, gfp_t gfp)
 
 void kunit_cleanup(struct kunit *test);
 
-void __printf(3, 4) kunit_printk(const char *level,
-				 const struct kunit *test,
-				 const char *fmt, ...);
+#define kunit_printk(lvl, test, fmt, ...) \
+	printk(lvl "\t# %s: " fmt, (test)->name, ##__VA_ARGS__)
 
 /**
  * kunit_info() - Prints an INFO level message associated with @test.
diff --git a/lib/kunit/test.c b/lib/kunit/test.c
index b2ca9b94c353..c83c0fa59cbd 100644
--- a/lib/kunit/test.c
+++ b/lib/kunit/test.c
@@ -16,36 +16,12 @@ static void kunit_set_failure(struct kunit *test)
 	WRITE_ONCE(test->success, false);
 }
 
-static int kunit_vprintk_emit(int level, const char *fmt, va_list args)
-{
-	return vprintk_emit(0, level, NULL, 0, fmt, args);
-}
-
-static int kunit_printk_emit(int level, const char *fmt, ...)
-{
-	va_list args;
-	int ret;
-
-	va_start(args, fmt);
-	ret = kunit_vprintk_emit(level, fmt, args);
-	va_end(args);
-
-	return ret;
-}
-
-static void kunit_vprintk(const struct kunit *test,
-			  const char *level,
-			  struct va_format *vaf)
-{
-	kunit_printk_emit(level[1] - '0', "\t# %s: %pV", test->name, vaf);
-}
-
 static void kunit_print_tap_version(void)
 {
 	static bool kunit_has_printed_tap_version;
 
 	if (!kunit_has_printed_tap_version) {
-		kunit_printk_emit(LOGLEVEL_INFO, "TAP version 14\n");
+		pr_info("TAP version 14\n");
 		kunit_has_printed_tap_version = true;
 	}
 }
@@ -64,10 +40,8 @@ static size_t kunit_test_cases_len(struct kunit_case *test_cases)
 static void kunit_print_subtest_start(struct kunit_suite *suite)
 {
 	kunit_print_tap_version();
-	kunit_printk_emit(LOGLEVEL_INFO, "\t# Subtest: %s\n", suite->name);
-	kunit_printk_emit(LOGLEVEL_INFO,
-			  "\t1..%zd\n",
-			  kunit_test_cases_len(suite->test_cases));
+	pr_info("\t# Subtest: %s\n", suite->name);
+	pr_info("\t1..%zd\n", kunit_test_cases_len(suite->test_cases));
 }
 
 static void kunit_print_ok_not_ok(bool should_indent,
@@ -87,9 +61,7 @@ static void kunit_print_ok_not_ok(bool should_indent,
 	else
 		ok_not_ok = "not ok";
 
-	kunit_printk_emit(LOGLEVEL_INFO,
-			  "%s%s %zd - %s\n",
-			  indent, ok_not_ok, test_number, description);
+	pr_info("%s%s %zd - %s\n", indent, ok_not_ok, test_number, description);
 }
 
 static bool kunit_suite_has_succeeded(struct kunit_suite *suite)
@@ -133,11 +105,11 @@ static void kunit_print_string_stream(struct kunit *test,
 		kunit_err(test,
 			  "Could not allocate buffer, dumping stream:\n");
 		list_for_each_entry(fragment, &stream->fragments, node) {
-			kunit_err(test, fragment->fragment);
+			kunit_err(test, "%s", fragment->fragment);
 		}
 		kunit_err(test, "\n");
 	} else {
-		kunit_err(test, buf);
+		kunit_err(test, "%s", buf);
 		kunit_kfree(test, buf);
 	}
 }
@@ -504,20 +476,3 @@ void kunit_cleanup(struct kunit *test)
 		kunit_resource_free(test, resource);
 	}
 }
-
-void kunit_printk(const char *level,
-		  const struct kunit *test,
-		  const char *fmt, ...)
-{
-	struct va_format vaf;
-	va_list args;
-
-	va_start(args, fmt);
-
-	vaf.fmt = fmt;
-	vaf.va = &args;
-
-	kunit_vprintk(test, level, &vaf);
-
-	va_end(args);
-}
-- 
2.23.0.351.gc4317032e6-goog

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

WARNING: multiple messages have this Message-ID (diff)
From: Brendan Higgins <brendanhiggins@google.com>
To: frowand.list@gmail.com, gregkh@linuxfoundation.org,
	jpoimboe@redhat.com, keescook@google.com,
	kieran.bingham@ideasonboard.com, mcgrof@kernel.org,
	peterz@infradead.org, robh@kernel.org, sboyd@kernel.org,
	shuah@kernel.org, tytso@mit.edu, yamada.masahiro@socionext.com
Cc: devicetree@vger.kernel.org, dri-devel@lists.freedesktop.org,
	kunit-dev@googlegroups.com, linux-doc@vger.kernel.org,
	linux-fsdevel@vger.kernel.org, linux-kbuild@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org,
	linux-nvdimm@lists.01.org, linux-um@lists.infradead.org,
	Alexander.Levin@microsoft.com, Tim.Bird@sony.com,
	amir73il@gmail.com, dan.carpenter@oracle.com, daniel@ffwll.ch,
	jdike@addtoit.com, joel@jms.id.au, julia.lawall@lip6.fr,
	khilman@baylibre.com, knut.omang@oracle.com, logang@deltatee.com,
	mpe@ellerman.id.au, pmladek@suse.com, rdunlap@infradead.org,
	richard@nod.at, rientjes@google.com, rostedt@goodmis.org,
	wfg@linux.intel.com, torvalds@linux-foundation.org,
	Brendan Higgins <brendanhiggins@google.com>,
	Stephen Rothwell <sfr@canb.auug.org.au>,
	Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>,
	Joe Perches <joe@perches.com>
Subject: [PATCH v16 19/19] kunit: fix failure to build without printk
Date: Fri, 20 Sep 2019 16:19:23 -0700	[thread overview]
Message-ID: <20190920231923.141900-20-brendanhiggins@google.com> (raw)
In-Reply-To: <20190920231923.141900-1-brendanhiggins@google.com>

Previously KUnit assumed that printk would always be present, which is
not a valid assumption to make. Fix that by removing call to
vprintk_emit, and calling printk directly.

This fixes a build error[1] reported by Randy.

For context this change comes after much discussion. My first stab[2] at
this was just to make the KUnit logging code compile out; however, it
was agreed that if we were going to use vprintk_emit, then vprintk_emit
should provide a no-op stub, which lead to my second attempt[3]. In
response to me trying to stub out vprintk_emit, Sergey Senozhatsky
suggested a way for me to remove our usage of vprintk_emit, which led to
my third attempt at solving this[4].

In my third version of this patch[4], I completely removed vprintk_emit,
as suggested by Sergey; however, there was a bit of debate over whether
Sergey's solution was the best. The debate arose due to Sergey's version
resulting in a checkpatch warning, which resulted in a debate over
correct printk usage. Joe Perches offered an alternative fix which was
somewhat less far reaching than what Sergey had suggested and
importantly relied on continuing to use %pV. Much of the debated
centered around whether %pV should be widely used, and whether Sergey's
version would result in object size bloat. Ultimately, we decided to go
with Sergey's version.

Reported-by: Randy Dunlap <rdunlap@infradead.org>
Link[1]: https://lore.kernel.org/linux-kselftest/c7229254-0d90-d90e-f3df-5b6d6fc0b51f@infradead.org/
Link[2]: https://lore.kernel.org/linux-kselftest/20190827174932.44177-1-brendanhiggins@google.com/
Link[3]: https://lore.kernel.org/linux-kselftest/20190827234835.234473-1-brendanhiggins@google.com/
Link[4]: https://lore.kernel.org/linux-kselftest/20190828093143.163302-1-brendanhiggins@google.com/
Cc: Stephen Rothwell <sfr@canb.auug.org.au>
Cc: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Cc: Joe Perches <joe@perches.com>
Cc: Tim.Bird@sony.com
Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
Acked-by: Randy Dunlap <rdunlap@infradead.org> # build-tested
Reviewed-by: Petr Mladek <pmladek@suse.com>
---
 include/kunit/test.h |  5 ++--
 lib/kunit/test.c     | 57 +++++---------------------------------------
 2 files changed, 8 insertions(+), 54 deletions(-)

diff --git a/include/kunit/test.h b/include/kunit/test.h
index 8b7eb03d4971..dba48304b3bd 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -339,9 +339,8 @@ static inline void *kunit_kzalloc(struct kunit *test, size_t size, gfp_t gfp)
 
 void kunit_cleanup(struct kunit *test);
 
-void __printf(3, 4) kunit_printk(const char *level,
-				 const struct kunit *test,
-				 const char *fmt, ...);
+#define kunit_printk(lvl, test, fmt, ...) \
+	printk(lvl "\t# %s: " fmt, (test)->name, ##__VA_ARGS__)
 
 /**
  * kunit_info() - Prints an INFO level message associated with @test.
diff --git a/lib/kunit/test.c b/lib/kunit/test.c
index b2ca9b94c353..c83c0fa59cbd 100644
--- a/lib/kunit/test.c
+++ b/lib/kunit/test.c
@@ -16,36 +16,12 @@ static void kunit_set_failure(struct kunit *test)
 	WRITE_ONCE(test->success, false);
 }
 
-static int kunit_vprintk_emit(int level, const char *fmt, va_list args)
-{
-	return vprintk_emit(0, level, NULL, 0, fmt, args);
-}
-
-static int kunit_printk_emit(int level, const char *fmt, ...)
-{
-	va_list args;
-	int ret;
-
-	va_start(args, fmt);
-	ret = kunit_vprintk_emit(level, fmt, args);
-	va_end(args);
-
-	return ret;
-}
-
-static void kunit_vprintk(const struct kunit *test,
-			  const char *level,
-			  struct va_format *vaf)
-{
-	kunit_printk_emit(level[1] - '0', "\t# %s: %pV", test->name, vaf);
-}
-
 static void kunit_print_tap_version(void)
 {
 	static bool kunit_has_printed_tap_version;
 
 	if (!kunit_has_printed_tap_version) {
-		kunit_printk_emit(LOGLEVEL_INFO, "TAP version 14\n");
+		pr_info("TAP version 14\n");
 		kunit_has_printed_tap_version = true;
 	}
 }
@@ -64,10 +40,8 @@ static size_t kunit_test_cases_len(struct kunit_case *test_cases)
 static void kunit_print_subtest_start(struct kunit_suite *suite)
 {
 	kunit_print_tap_version();
-	kunit_printk_emit(LOGLEVEL_INFO, "\t# Subtest: %s\n", suite->name);
-	kunit_printk_emit(LOGLEVEL_INFO,
-			  "\t1..%zd\n",
-			  kunit_test_cases_len(suite->test_cases));
+	pr_info("\t# Subtest: %s\n", suite->name);
+	pr_info("\t1..%zd\n", kunit_test_cases_len(suite->test_cases));
 }
 
 static void kunit_print_ok_not_ok(bool should_indent,
@@ -87,9 +61,7 @@ static void kunit_print_ok_not_ok(bool should_indent,
 	else
 		ok_not_ok = "not ok";
 
-	kunit_printk_emit(LOGLEVEL_INFO,
-			  "%s%s %zd - %s\n",
-			  indent, ok_not_ok, test_number, description);
+	pr_info("%s%s %zd - %s\n", indent, ok_not_ok, test_number, description);
 }
 
 static bool kunit_suite_has_succeeded(struct kunit_suite *suite)
@@ -133,11 +105,11 @@ static void kunit_print_string_stream(struct kunit *test,
 		kunit_err(test,
 			  "Could not allocate buffer, dumping stream:\n");
 		list_for_each_entry(fragment, &stream->fragments, node) {
-			kunit_err(test, fragment->fragment);
+			kunit_err(test, "%s", fragment->fragment);
 		}
 		kunit_err(test, "\n");
 	} else {
-		kunit_err(test, buf);
+		kunit_err(test, "%s", buf);
 		kunit_kfree(test, buf);
 	}
 }
@@ -504,20 +476,3 @@ void kunit_cleanup(struct kunit *test)
 		kunit_resource_free(test, resource);
 	}
 }
-
-void kunit_printk(const char *level,
-		  const struct kunit *test,
-		  const char *fmt, ...)
-{
-	struct va_format vaf;
-	va_list args;
-
-	va_start(args, fmt);
-
-	vaf.fmt = fmt;
-	vaf.va = &args;
-
-	kunit_vprintk(test, level, &vaf);
-
-	va_end(args);
-}
-- 
2.23.0.351.gc4317032e6-goog


WARNING: multiple messages have this Message-ID (diff)
From: Brendan Higgins <brendanhiggins@google.com>
To: frowand.list@gmail.com, gregkh@linuxfoundation.org,
	jpoimboe@redhat.com, keescook@google.com,
	kieran.bingham@ideasonboard.com, mcgrof@kernel.org,
	peterz@infradead.org, robh@kernel.org, sboyd@kernel.org,
	shuah@kernel.org, tytso@mit.edu, yamada.masahiro@socionext.com
Cc: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>,
	pmladek@suse.com, linux-doc@vger.kernel.org, amir73il@gmail.com,
	Brendan Higgins <brendanhiggins@google.com>,
	dri-devel@lists.freedesktop.org, Alexander.Levin@microsoft.com,
	linux-kselftest@vger.kernel.org,
	Stephen Rothwell <sfr@canb.auug.org.au>,
	linux-nvdimm@lists.01.org, khilman@baylibre.com,
	knut.omang@oracle.com, wfg@linux.intel.com, joel@jms.id.au,
	rientjes@google.com, jdike@addtoit.com, dan.carpenter@oracle.com,
	devicetree@vger.kernel.org, linux-kbuild@vger.kernel.org,
	Tim.Bird@sony.com, linux-um@lists.infradead.org,
	rostedt@goodmis.org, julia.lawall@lip6.fr,
	kunit-dev@googlegroups.com, richard@nod.at,
	torvalds@linux-foundation.org, rdunlap@infradead.org,
	linux-kernel@vger.kernel.org, mpe@ellerman.id.au,
	linux-fsdevel@vger.kernel.org, Joe Perches <joe@perches.com>,
	logang@deltatee.com
Subject: [PATCH v16 19/19] kunit: fix failure to build without printk
Date: Fri, 20 Sep 2019 16:19:23 -0700	[thread overview]
Message-ID: <20190920231923.141900-20-brendanhiggins@google.com> (raw)
In-Reply-To: <20190920231923.141900-1-brendanhiggins@google.com>

Previously KUnit assumed that printk would always be present, which is
not a valid assumption to make. Fix that by removing call to
vprintk_emit, and calling printk directly.

This fixes a build error[1] reported by Randy.

For context this change comes after much discussion. My first stab[2] at
this was just to make the KUnit logging code compile out; however, it
was agreed that if we were going to use vprintk_emit, then vprintk_emit
should provide a no-op stub, which lead to my second attempt[3]. In
response to me trying to stub out vprintk_emit, Sergey Senozhatsky
suggested a way for me to remove our usage of vprintk_emit, which led to
my third attempt at solving this[4].

In my third version of this patch[4], I completely removed vprintk_emit,
as suggested by Sergey; however, there was a bit of debate over whether
Sergey's solution was the best. The debate arose due to Sergey's version
resulting in a checkpatch warning, which resulted in a debate over
correct printk usage. Joe Perches offered an alternative fix which was
somewhat less far reaching than what Sergey had suggested and
importantly relied on continuing to use %pV. Much of the debated
centered around whether %pV should be widely used, and whether Sergey's
version would result in object size bloat. Ultimately, we decided to go
with Sergey's version.

Reported-by: Randy Dunlap <rdunlap@infradead.org>
Link[1]: https://lore.kernel.org/linux-kselftest/c7229254-0d90-d90e-f3df-5b6d6fc0b51f@infradead.org/
Link[2]: https://lore.kernel.org/linux-kselftest/20190827174932.44177-1-brendanhiggins@google.com/
Link[3]: https://lore.kernel.org/linux-kselftest/20190827234835.234473-1-brendanhiggins@google.com/
Link[4]: https://lore.kernel.org/linux-kselftest/20190828093143.163302-1-brendanhiggins@google.com/
Cc: Stephen Rothwell <sfr@canb.auug.org.au>
Cc: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Cc: Joe Perches <joe@perches.com>
Cc: Tim.Bird@sony.com
Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
Acked-by: Randy Dunlap <rdunlap@infradead.org> # build-tested
Reviewed-by: Petr Mladek <pmladek@suse.com>
---
 include/kunit/test.h |  5 ++--
 lib/kunit/test.c     | 57 +++++---------------------------------------
 2 files changed, 8 insertions(+), 54 deletions(-)

diff --git a/include/kunit/test.h b/include/kunit/test.h
index 8b7eb03d4971..dba48304b3bd 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -339,9 +339,8 @@ static inline void *kunit_kzalloc(struct kunit *test, size_t size, gfp_t gfp)
 
 void kunit_cleanup(struct kunit *test);
 
-void __printf(3, 4) kunit_printk(const char *level,
-				 const struct kunit *test,
-				 const char *fmt, ...);
+#define kunit_printk(lvl, test, fmt, ...) \
+	printk(lvl "\t# %s: " fmt, (test)->name, ##__VA_ARGS__)
 
 /**
  * kunit_info() - Prints an INFO level message associated with @test.
diff --git a/lib/kunit/test.c b/lib/kunit/test.c
index b2ca9b94c353..c83c0fa59cbd 100644
--- a/lib/kunit/test.c
+++ b/lib/kunit/test.c
@@ -16,36 +16,12 @@ static void kunit_set_failure(struct kunit *test)
 	WRITE_ONCE(test->success, false);
 }
 
-static int kunit_vprintk_emit(int level, const char *fmt, va_list args)
-{
-	return vprintk_emit(0, level, NULL, 0, fmt, args);
-}
-
-static int kunit_printk_emit(int level, const char *fmt, ...)
-{
-	va_list args;
-	int ret;
-
-	va_start(args, fmt);
-	ret = kunit_vprintk_emit(level, fmt, args);
-	va_end(args);
-
-	return ret;
-}
-
-static void kunit_vprintk(const struct kunit *test,
-			  const char *level,
-			  struct va_format *vaf)
-{
-	kunit_printk_emit(level[1] - '0', "\t# %s: %pV", test->name, vaf);
-}
-
 static void kunit_print_tap_version(void)
 {
 	static bool kunit_has_printed_tap_version;
 
 	if (!kunit_has_printed_tap_version) {
-		kunit_printk_emit(LOGLEVEL_INFO, "TAP version 14\n");
+		pr_info("TAP version 14\n");
 		kunit_has_printed_tap_version = true;
 	}
 }
@@ -64,10 +40,8 @@ static size_t kunit_test_cases_len(struct kunit_case *test_cases)
 static void kunit_print_subtest_start(struct kunit_suite *suite)
 {
 	kunit_print_tap_version();
-	kunit_printk_emit(LOGLEVEL_INFO, "\t# Subtest: %s\n", suite->name);
-	kunit_printk_emit(LOGLEVEL_INFO,
-			  "\t1..%zd\n",
-			  kunit_test_cases_len(suite->test_cases));
+	pr_info("\t# Subtest: %s\n", suite->name);
+	pr_info("\t1..%zd\n", kunit_test_cases_len(suite->test_cases));
 }
 
 static void kunit_print_ok_not_ok(bool should_indent,
@@ -87,9 +61,7 @@ static void kunit_print_ok_not_ok(bool should_indent,
 	else
 		ok_not_ok = "not ok";
 
-	kunit_printk_emit(LOGLEVEL_INFO,
-			  "%s%s %zd - %s\n",
-			  indent, ok_not_ok, test_number, description);
+	pr_info("%s%s %zd - %s\n", indent, ok_not_ok, test_number, description);
 }
 
 static bool kunit_suite_has_succeeded(struct kunit_suite *suite)
@@ -133,11 +105,11 @@ static void kunit_print_string_stream(struct kunit *test,
 		kunit_err(test,
 			  "Could not allocate buffer, dumping stream:\n");
 		list_for_each_entry(fragment, &stream->fragments, node) {
-			kunit_err(test, fragment->fragment);
+			kunit_err(test, "%s", fragment->fragment);
 		}
 		kunit_err(test, "\n");
 	} else {
-		kunit_err(test, buf);
+		kunit_err(test, "%s", buf);
 		kunit_kfree(test, buf);
 	}
 }
@@ -504,20 +476,3 @@ void kunit_cleanup(struct kunit *test)
 		kunit_resource_free(test, resource);
 	}
 }
-
-void kunit_printk(const char *level,
-		  const struct kunit *test,
-		  const char *fmt, ...)
-{
-	struct va_format vaf;
-	va_list args;
-
-	va_start(args, fmt);
-
-	vaf.fmt = fmt;
-	vaf.va = &args;
-
-	kunit_vprintk(test, level, &vaf);
-
-	va_end(args);
-}
-- 
2.23.0.351.gc4317032e6-goog

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

WARNING: multiple messages have this Message-ID (diff)
From: Brendan Higgins <brendanhiggins@google.com>
To: frowand.list@gmail.com, gregkh@linuxfoundation.org,
	jpoimboe@redhat.com, keescook@google.com,
	kieran.bingham@ideasonboard.com, mcgrof@kernel.org,
	peterz@infradead.org, robh@kernel.org, sboyd@kernel.org,
	shuah@kernel.org, tytso@mit.edu, yamada.masahiro@socionext.com
Cc: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>,
	pmladek@suse.com, linux-doc@vger.kernel.org, amir73il@gmail.com,
	Brendan Higgins <brendanhiggins@google.com>,
	dri-devel@lists.freedesktop.org, Alexander.Levin@microsoft.com,
	linux-kselftest@vger.kernel.org,
	Stephen Rothwell <sfr@canb.auug.org.au>,
	linux-nvdimm@lists.01.org, khilman@baylibre.com,
	knut.omang@oracle.com, wfg@linux.intel.com, joel@jms.id.au,
	rientjes@google.com, jdike@addtoit.com, dan.carpenter@oracle.com,
	devicetree@vger.kernel.org, linux-kbuild@vger.kernel.org,
	Tim.Bird@sony.com, linux-um@lists.infradead.org,
	rostedt@goodmis.org, julia.lawall@lip6.fr,
	kunit-dev@googlegroups.com, richard@nod.at,
	torvalds@linux-foundation.org, rdunlap@infradead.org,
	linux-kernel@vger.kernel.org, daniel@ffwll.ch,
	mpe@ellerman.id.au, linux-fsdevel@vger.kernel.org,
	Joe Perches <joe@perches.com>,
	logang@deltatee.com
Subject: [PATCH v16 19/19] kunit: fix failure to build without printk
Date: Fri, 20 Sep 2019 16:19:23 -0700	[thread overview]
Message-ID: <20190920231923.141900-20-brendanhiggins@google.com> (raw)
In-Reply-To: <20190920231923.141900-1-brendanhiggins@google.com>

Previously KUnit assumed that printk would always be present, which is
not a valid assumption to make. Fix that by removing call to
vprintk_emit, and calling printk directly.

This fixes a build error[1] reported by Randy.

For context this change comes after much discussion. My first stab[2] at
this was just to make the KUnit logging code compile out; however, it
was agreed that if we were going to use vprintk_emit, then vprintk_emit
should provide a no-op stub, which lead to my second attempt[3]. In
response to me trying to stub out vprintk_emit, Sergey Senozhatsky
suggested a way for me to remove our usage of vprintk_emit, which led to
my third attempt at solving this[4].

In my third version of this patch[4], I completely removed vprintk_emit,
as suggested by Sergey; however, there was a bit of debate over whether
Sergey's solution was the best. The debate arose due to Sergey's version
resulting in a checkpatch warning, which resulted in a debate over
correct printk usage. Joe Perches offered an alternative fix which was
somewhat less far reaching than what Sergey had suggested and
importantly relied on continuing to use %pV. Much of the debated
centered around whether %pV should be widely used, and whether Sergey's
version would result in object size bloat. Ultimately, we decided to go
with Sergey's version.

Reported-by: Randy Dunlap <rdunlap@infradead.org>
Link[1]: https://lore.kernel.org/linux-kselftest/c7229254-0d90-d90e-f3df-5b6d6fc0b51f@infradead.org/
Link[2]: https://lore.kernel.org/linux-kselftest/20190827174932.44177-1-brendanhiggins@google.com/
Link[3]: https://lore.kernel.org/linux-kselftest/20190827234835.234473-1-brendanhiggins@google.com/
Link[4]: https://lore.kernel.org/linux-kselftest/20190828093143.163302-1-brendanhiggins@google.com/
Cc: Stephen Rothwell <sfr@canb.auug.org.au>
Cc: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Cc: Joe Perches <joe@perches.com>
Cc: Tim.Bird@sony.com
Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
Acked-by: Randy Dunlap <rdunlap@infradead.org> # build-tested
Reviewed-by: Petr Mladek <pmladek@suse.com>
---
 include/kunit/test.h |  5 ++--
 lib/kunit/test.c     | 57 +++++---------------------------------------
 2 files changed, 8 insertions(+), 54 deletions(-)

diff --git a/include/kunit/test.h b/include/kunit/test.h
index 8b7eb03d4971..dba48304b3bd 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -339,9 +339,8 @@ static inline void *kunit_kzalloc(struct kunit *test, size_t size, gfp_t gfp)
 
 void kunit_cleanup(struct kunit *test);
 
-void __printf(3, 4) kunit_printk(const char *level,
-				 const struct kunit *test,
-				 const char *fmt, ...);
+#define kunit_printk(lvl, test, fmt, ...) \
+	printk(lvl "\t# %s: " fmt, (test)->name, ##__VA_ARGS__)
 
 /**
  * kunit_info() - Prints an INFO level message associated with @test.
diff --git a/lib/kunit/test.c b/lib/kunit/test.c
index b2ca9b94c353..c83c0fa59cbd 100644
--- a/lib/kunit/test.c
+++ b/lib/kunit/test.c
@@ -16,36 +16,12 @@ static void kunit_set_failure(struct kunit *test)
 	WRITE_ONCE(test->success, false);
 }
 
-static int kunit_vprintk_emit(int level, const char *fmt, va_list args)
-{
-	return vprintk_emit(0, level, NULL, 0, fmt, args);
-}
-
-static int kunit_printk_emit(int level, const char *fmt, ...)
-{
-	va_list args;
-	int ret;
-
-	va_start(args, fmt);
-	ret = kunit_vprintk_emit(level, fmt, args);
-	va_end(args);
-
-	return ret;
-}
-
-static void kunit_vprintk(const struct kunit *test,
-			  const char *level,
-			  struct va_format *vaf)
-{
-	kunit_printk_emit(level[1] - '0', "\t# %s: %pV", test->name, vaf);
-}
-
 static void kunit_print_tap_version(void)
 {
 	static bool kunit_has_printed_tap_version;
 
 	if (!kunit_has_printed_tap_version) {
-		kunit_printk_emit(LOGLEVEL_INFO, "TAP version 14\n");
+		pr_info("TAP version 14\n");
 		kunit_has_printed_tap_version = true;
 	}
 }
@@ -64,10 +40,8 @@ static size_t kunit_test_cases_len(struct kunit_case *test_cases)
 static void kunit_print_subtest_start(struct kunit_suite *suite)
 {
 	kunit_print_tap_version();
-	kunit_printk_emit(LOGLEVEL_INFO, "\t# Subtest: %s\n", suite->name);
-	kunit_printk_emit(LOGLEVEL_INFO,
-			  "\t1..%zd\n",
-			  kunit_test_cases_len(suite->test_cases));
+	pr_info("\t# Subtest: %s\n", suite->name);
+	pr_info("\t1..%zd\n", kunit_test_cases_len(suite->test_cases));
 }
 
 static void kunit_print_ok_not_ok(bool should_indent,
@@ -87,9 +61,7 @@ static void kunit_print_ok_not_ok(bool should_indent,
 	else
 		ok_not_ok = "not ok";
 
-	kunit_printk_emit(LOGLEVEL_INFO,
-			  "%s%s %zd - %s\n",
-			  indent, ok_not_ok, test_number, description);
+	pr_info("%s%s %zd - %s\n", indent, ok_not_ok, test_number, description);
 }
 
 static bool kunit_suite_has_succeeded(struct kunit_suite *suite)
@@ -133,11 +105,11 @@ static void kunit_print_string_stream(struct kunit *test,
 		kunit_err(test,
 			  "Could not allocate buffer, dumping stream:\n");
 		list_for_each_entry(fragment, &stream->fragments, node) {
-			kunit_err(test, fragment->fragment);
+			kunit_err(test, "%s", fragment->fragment);
 		}
 		kunit_err(test, "\n");
 	} else {
-		kunit_err(test, buf);
+		kunit_err(test, "%s", buf);
 		kunit_kfree(test, buf);
 	}
 }
@@ -504,20 +476,3 @@ void kunit_cleanup(struct kunit *test)
 		kunit_resource_free(test, resource);
 	}
 }
-
-void kunit_printk(const char *level,
-		  const struct kunit *test,
-		  const char *fmt, ...)
-{
-	struct va_format vaf;
-	va_list args;
-
-	va_start(args, fmt);
-
-	vaf.fmt = fmt;
-	vaf.va = &args;
-
-	kunit_vprintk(test, level, &vaf);
-
-	va_end(args);
-}
-- 
2.23.0.351.gc4317032e6-goog


_______________________________________________
linux-um mailing list
linux-um@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-um


  parent reply	other threads:[~2019-09-20 23:49 UTC|newest]

Thread overview: 86+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-20 23:19 [PATCH v16 00/19] kunit: introduce KUnit, the Linux kernel unit testing framework Brendan Higgins
2019-09-20 23:19 ` Brendan Higgins
2019-09-20 23:19 ` Brendan Higgins
2019-09-20 23:19 ` [PATCH v16 01/19] kunit: test: add KUnit test runner core Brendan Higgins
2019-09-20 23:19   ` Brendan Higgins
2019-09-20 23:19   ` Brendan Higgins
2019-09-20 23:19   ` Brendan Higgins
2019-09-20 23:19 ` [PATCH v16 02/19] kunit: test: add test resource management API Brendan Higgins
2019-09-20 23:19   ` Brendan Higgins
2019-09-20 23:19   ` Brendan Higgins
2019-09-20 23:19   ` Brendan Higgins
2019-09-20 23:19 ` [PATCH v16 03/19] kunit: test: add string_stream a std::stream like string builder Brendan Higgins
2019-09-20 23:19   ` Brendan Higgins
2019-09-20 23:19   ` Brendan Higgins
2019-09-20 23:19 ` [PATCH v16 04/19] kunit: test: add assertion printing library Brendan Higgins
2019-09-20 23:19   ` Brendan Higgins
2019-09-20 23:19   ` Brendan Higgins
2019-09-20 23:19   ` Brendan Higgins
2019-09-20 23:19 ` [PATCH v16 05/19] kunit: test: add the concept of expectations Brendan Higgins
2019-09-20 23:19   ` Brendan Higgins
2019-09-20 23:19   ` Brendan Higgins
2019-09-20 23:19   ` Brendan Higgins
2019-09-20 23:19 ` [PATCH v16 06/19] lib: enable building KUnit in lib/ Brendan Higgins
2019-09-20 23:19   ` Brendan Higgins
2019-09-20 23:19   ` Brendan Higgins
2019-09-20 23:19   ` Brendan Higgins
2019-09-20 23:35   ` Stephen Boyd
2019-09-20 23:35     ` Stephen Boyd
2019-09-20 23:35     ` Stephen Boyd
2019-09-20 23:35     ` Stephen Boyd
2019-09-20 23:35     ` Stephen Boyd
2019-09-20 23:44     ` Brendan Higgins
2019-09-20 23:44       ` Brendan Higgins
2019-09-20 23:44       ` Brendan Higgins
2019-09-20 23:44       ` Brendan Higgins
2019-09-20 23:44       ` Brendan Higgins
2019-09-20 23:52       ` Brendan Higgins
2019-09-20 23:52         ` Brendan Higgins
2019-09-20 23:52         ` Brendan Higgins
2019-09-20 23:52         ` Brendan Higgins
2019-09-20 23:52         ` Brendan Higgins
2019-09-20 23:19 ` [PATCH v16 07/19] kunit: test: add initial tests Brendan Higgins
2019-09-20 23:19   ` Brendan Higgins
2019-09-20 23:19   ` Brendan Higgins
2019-09-20 23:19 ` [PATCH v16 08/19] objtool: add kunit_try_catch_throw to the noreturn list Brendan Higgins
2019-09-20 23:19   ` Brendan Higgins
2019-09-20 23:19   ` Brendan Higgins
2019-09-20 23:19 ` [PATCH v16 09/19] kunit: test: add support for test abort Brendan Higgins
2019-09-20 23:19   ` Brendan Higgins
2019-09-20 23:19   ` Brendan Higgins
2019-09-20 23:19   ` Brendan Higgins
2019-09-20 23:19 ` [PATCH v16 10/19] kunit: test: add tests for kunit " Brendan Higgins
2019-09-20 23:19   ` Brendan Higgins
2019-09-20 23:19   ` Brendan Higgins
2019-09-20 23:19 ` [PATCH v16 11/19] kunit: test: add the concept of assertions Brendan Higgins
2019-09-20 23:19   ` Brendan Higgins
2019-09-20 23:19   ` Brendan Higgins
2019-09-20 23:19 ` [PATCH v16 12/19] kunit: test: add tests for KUnit managed resources Brendan Higgins
2019-09-20 23:19   ` Brendan Higgins
2019-09-20 23:19   ` Brendan Higgins
2019-09-20 23:19   ` Brendan Higgins
2019-09-20 23:19 ` [PATCH v16 13/19] kunit: tool: add Python wrappers for running KUnit tests Brendan Higgins
2019-09-20 23:19   ` Brendan Higgins
2019-09-20 23:19   ` Brendan Higgins
2019-09-20 23:19 ` [PATCH v16 14/19] kunit: defconfig: add defconfigs for building " Brendan Higgins
2019-09-20 23:19   ` Brendan Higgins
2019-09-20 23:19   ` Brendan Higgins
2019-09-20 23:19   ` Brendan Higgins
2019-09-20 23:19 ` [PATCH v16 15/19] Documentation: kunit: add documentation for KUnit Brendan Higgins
2019-09-20 23:19   ` Brendan Higgins
2019-09-20 23:19   ` Brendan Higgins
2019-09-20 23:19   ` Brendan Higgins
2019-09-20 23:19 ` [PATCH v16 16/19] MAINTAINERS: add entry for KUnit the unit testing framework Brendan Higgins
2019-09-20 23:19   ` Brendan Higgins
2019-09-20 23:19   ` Brendan Higgins
2019-09-20 23:19 ` [PATCH v16 17/19] kernel/sysctl-test: Add null pointer test for sysctl.c:proc_dointvec() Brendan Higgins
2019-09-20 23:19   ` Brendan Higgins
2019-09-20 23:19   ` Brendan Higgins
2019-09-20 23:19   ` Brendan Higgins
2019-09-20 23:19 ` [PATCH v16 18/19] MAINTAINERS: add proc sysctl KUnit test to PROC SYSCTL section Brendan Higgins
2019-09-20 23:19   ` Brendan Higgins
2019-09-20 23:19   ` Brendan Higgins
2019-09-20 23:19 ` Brendan Higgins [this message]
2019-09-20 23:19   ` [PATCH v16 19/19] kunit: fix failure to build without printk Brendan Higgins
2019-09-20 23:19   ` Brendan Higgins
2019-09-20 23:19   ` Brendan Higgins

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190920231923.141900-20-brendanhiggins@google.com \
    --to=brendanhiggins@google.com \
    --cc=Alexander.Levin@microsoft.com \
    --cc=Tim.Bird@sony.com \
    --cc=amir73il@gmail.com \
    --cc=dan.carpenter@oracle.com \
    --cc=daniel@ffwll.ch \
    --cc=devicetree@vger.kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=frowand.list@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jdike@addtoit.com \
    --cc=joe@perches.com \
    --cc=joel@jms.id.au \
    --cc=jpoimboe@redhat.com \
    --cc=julia.lawall@lip6.fr \
    --cc=keescook@google.com \
    --cc=khilman@baylibre.com \
    --cc=kieran.bingham@ideasonboard.com \
    --cc=knut.omang@oracle.com \
    --cc=kunit-dev@googlegroups.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-nvdimm@lists.01.org \
    --cc=linux-um@lists.infradead.org \
    --cc=mcgrof@kernel.org \
    --cc=mpe@ellerman.id.au \
    --cc=peterz@infradead.org \
    --cc=pmladek@suse.com \
    --cc=rdunlap@infradead.org \
    --cc=richard@nod.at \
    --cc=rientjes@google.com \
    --cc=robh@kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=sboyd@kernel.org \
    --cc=sergey.senozhatsky.work@gmail.com \
    --cc=sfr@canb.auug.org.au \
    --cc=shuah@kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=tytso@mit.edu \
    --cc=wfg@linux.intel.com \
    --cc=yamada.masahiro@socionext.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.