Skip to content

Commit

Permalink
kunit: drop assumption in kunit-log-test about current suite
Browse files Browse the repository at this point in the history
This test assumes that the declared kunit_suite object is the exact one
which is being executed, which KUnit will not guarantee [1].

Specifically, `suite->log` is not initialized until a suite object is
executed. So if KUnit makes a copy of the suite and runs that instead,
this test dereferences an invalid pointer and (hopefully) segfaults.

N.B. since we no longer assume this, we can no longer verify that
`suite->log` is *not* allocated during normal execution.

An alternative to this patch that would allow us to test that would
require exposing an API for the current test to get its current suite.
Exposing that for one internal kunit test seems like overkill, and
grants users more footguns (e.g. reusing a test case in multiple suites
and changing behavior based on the suite name, dynamically modifying the
setup/cleanup funcs, storing/reading stuff out of the suite->log, etc.).

[1] In a subsequent patch, KUnit will allow running subsets of test
cases within a suite by making a copy of the suite w/ the filtered test
list. But there are other reasons KUnit might execute a copy, e.g. if it
ever wants to support parallel execution of different suites, recovering
from errors and restarting suites

Signed-off-by: Daniel Latypov <[email protected]>
Reviewed-by: Brendan Higgins <[email protected]>
Signed-off-by: Shuah Khan <[email protected]>
  • Loading branch information
dlatypov authored and shuahkh committed Oct 19, 2021
1 parent 519d819 commit b7cbaef
Showing 1 changed file with 8 additions and 6 deletions.
14 changes: 8 additions & 6 deletions lib/kunit/kunit-test.c
Original file line number Diff line number Diff line change
Expand Up @@ -415,25 +415,27 @@ static struct kunit_suite kunit_log_test_suite = {

static void kunit_log_test(struct kunit *test)
{
struct kunit_suite *suite = &kunit_log_test_suite;
struct kunit_suite suite;

suite.log = kunit_kzalloc(test, KUNIT_LOG_SIZE, GFP_KERNEL);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, suite.log);

kunit_log(KERN_INFO, test, "put this in log.");
kunit_log(KERN_INFO, test, "this too.");
kunit_log(KERN_INFO, suite, "add to suite log.");
kunit_log(KERN_INFO, suite, "along with this.");
kunit_log(KERN_INFO, &suite, "add to suite log.");
kunit_log(KERN_INFO, &suite, "along with this.");

#ifdef CONFIG_KUNIT_DEBUGFS
KUNIT_EXPECT_NOT_ERR_OR_NULL(test,
strstr(test->log, "put this in log."));
KUNIT_EXPECT_NOT_ERR_OR_NULL(test,
strstr(test->log, "this too."));
KUNIT_EXPECT_NOT_ERR_OR_NULL(test,
strstr(suite->log, "add to suite log."));
strstr(suite.log, "add to suite log."));
KUNIT_EXPECT_NOT_ERR_OR_NULL(test,
strstr(suite->log, "along with this."));
strstr(suite.log, "along with this."));
#else
KUNIT_EXPECT_PTR_EQ(test, test->log, (char *)NULL);
KUNIT_EXPECT_PTR_EQ(test, suite->log, (char *)NULL);
#endif
}

Expand Down

0 comments on commit b7cbaef

Please sign in to comment.