From: Daniel Latypov <dlatypov@google.com>
To: brendanhiggins@google.com, davidgow@google.com
Cc: linux-kernel@vger.kernel.org, kunit-dev@googlegroups.com,
linux-kselftest@vger.kernel.org, skhan@linuxfoundation.org,
torvalds@linux-foundation.org,
Daniel Latypov <dlatypov@google.com>
Subject: [PATCH v3 2/6] kunit: move check if assertion passed into the macros
Date: Thu, 13 Jan 2022 08:59:27 -0800 [thread overview]
Message-ID: <20220113165931.451305-3-dlatypov@google.com> (raw)
In-Reply-To: <20220113165931.451305-1-dlatypov@google.com>
Currently the code always calls kunit_do_assertion() even though it does
nothing when `pass` is true.
This change moves the `if(!(pass))` check into the macro instead
and renames the function to kunit_do_failed_assertion().
I feel this a bit easier to read and understand.
This has the potential upside of avoiding a function call that does
nothing most of the time (assuming your tests are passing) but comes
with the downside of generating a bit more code and branches. We try to
mitigate the branches by tagging them with `unlikely()`.
This also means we don't have to initialize structs that we don't need,
which will become a tiny bit more expensive if we switch over to using
static variables to try and reduce stack usage. (There's runtime code
to check if the variable has been initialized yet or not).
Signed-off-by: Daniel Latypov <dlatypov@google.com>
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
Reviewed-by: David Gow <davidgow@google.com>
---
include/kunit/test.h | 21 +++++++++++----------
lib/kunit/test.c | 13 ++++---------
2 files changed, 15 insertions(+), 19 deletions(-)
diff --git a/include/kunit/test.h b/include/kunit/test.h
index b26400731c02..12cabd15449a 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -12,6 +12,7 @@
#include <kunit/assert.h>
#include <kunit/try-catch.h>
+#include <linux/compiler.h>
#include <linux/container_of.h>
#include <linux/err.h>
#include <linux/init.h>
@@ -770,18 +771,18 @@ void __printf(2, 3) kunit_log_append(char *log, const char *fmt, ...);
*/
#define KUNIT_SUCCEED(test) do {} while (0)
-void kunit_do_assertion(struct kunit *test,
- struct kunit_assert *assert,
- bool pass,
- const char *fmt, ...);
+void kunit_do_failed_assertion(struct kunit *test,
+ struct kunit_assert *assert,
+ const char *fmt, ...);
#define KUNIT_ASSERTION(test, pass, assert_class, INITIALIZER, fmt, ...) do { \
- struct assert_class __assertion = INITIALIZER; \
- kunit_do_assertion(test, \
- &__assertion.assert, \
- pass, \
- fmt, \
- ##__VA_ARGS__); \
+ if (unlikely(!(pass))) { \
+ struct assert_class __assertion = INITIALIZER; \
+ kunit_do_failed_assertion(test, \
+ &__assertion.assert, \
+ fmt, \
+ ##__VA_ARGS__); \
+ } \
} while (0)
diff --git a/lib/kunit/test.c b/lib/kunit/test.c
index c7ed4aabec04..3a52c321c280 100644
--- a/lib/kunit/test.c
+++ b/lib/kunit/test.c
@@ -275,16 +275,11 @@ static void __noreturn kunit_abort(struct kunit *test)
WARN_ONCE(true, "Throw could not abort from test!\n");
}
-void kunit_do_assertion(struct kunit *test,
- struct kunit_assert *assert,
- bool pass,
- const char *fmt, ...)
+void kunit_do_failed_assertion(struct kunit *test,
+ struct kunit_assert *assert,
+ const char *fmt, ...)
{
va_list args;
-
- if (pass)
- return;
-
va_start(args, fmt);
assert->message.fmt = fmt;
@@ -297,7 +292,7 @@ void kunit_do_assertion(struct kunit *test,
if (assert->type == KUNIT_ASSERTION)
kunit_abort(test);
}
-EXPORT_SYMBOL_GPL(kunit_do_assertion);
+EXPORT_SYMBOL_GPL(kunit_do_failed_assertion);
void kunit_init_test(struct kunit *test, const char *name, char *log)
{
--
2.34.1.703.g22d0c6ccf7-goog
next prev parent reply other threads:[~2022-01-13 16:59 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-13 16:59 [PATCH v3 0/6] kunit: refactor assertions to use less stack Daniel Latypov
2022-01-13 16:59 ` [PATCH v3 1/6] kunit: add example test case showing off all the expect macros Daniel Latypov
2022-01-13 16:59 ` Daniel Latypov [this message]
2022-01-13 16:59 ` [PATCH v3 3/6] kunit: drop unused kunit* field in kunit_assert Daniel Latypov
2022-01-13 16:59 ` [PATCH v3 4/6] kunit: factor out kunit_base_assert_format() call into kunit_fail() Daniel Latypov
2022-01-21 23:01 ` Brendan Higgins
2022-01-13 16:59 ` [PATCH v3 5/6] kunit: split out part of kunit_assert into a static const Daniel Latypov
2022-01-13 16:59 ` [PATCH v3 6/6] kunit: drop unused assert_type from kunit_assert and clean up macros Daniel Latypov
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=20220113165931.451305-3-dlatypov@google.com \
--to=dlatypov@google.com \
--cc=brendanhiggins@google.com \
--cc=davidgow@google.com \
--cc=kunit-dev@googlegroups.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=skhan@linuxfoundation.org \
--cc=torvalds@linux-foundation.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®