A few of the lists used in the linked-list KUnit tests (the for_each_entry{,_reverse} tests) are declared 'static', and so are not-reinitialised if the test runs multiple times. This was not a problem when KUnit tests were run once on startup, but when tests are able to be run manually (e.g. from debugfs[1]), this is no longer the case.
Making these lists no longer 'static' causes the lists to be reinitialised, and the test passes each time it is run. While there may be some value in testing that initialising static lists works, the for_each_entry_* tests are unlikely to be the right place for it.
Signed-off-by: David Gow davidgow@google.com --- lib/list-test.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/list-test.c b/lib/list-test.c index 76babb1df889..ee09505df16f 100644 --- a/lib/list-test.c +++ b/lib/list-test.c @@ -659,7 +659,7 @@ static void list_test_list_for_each_prev_safe(struct kunit *test) static void list_test_list_for_each_entry(struct kunit *test) { struct list_test_struct entries[5], *cur; - static LIST_HEAD(list); + LIST_HEAD(list); int i = 0;
for (i = 0; i < 5; ++i) { @@ -680,7 +680,7 @@ static void list_test_list_for_each_entry(struct kunit *test) static void list_test_list_for_each_entry_reverse(struct kunit *test) { struct list_test_struct entries[5], *cur; - static LIST_HEAD(list); + LIST_HEAD(list); int i = 0;
for (i = 0; i < 5; ++i) {
On Fri, Jan 24, 2020 at 11:46 AM David Gow davidgow@google.com wrote:
A few of the lists used in the linked-list KUnit tests (the for_each_entry{,_reverse} tests) are declared 'static', and so are not-reinitialised if the test runs multiple times. This was not a problem when KUnit tests were run once on startup, but when tests are able to be run manually (e.g. from debugfs[1]), this is no longer the case.
Making these lists no longer 'static' causes the lists to be reinitialised, and the test passes each time it is run. While there may be some value in testing that initialising static lists works, the for_each_entry_* tests are unlikely to be the right place for it.
Oh good, I am glad we are getting rid of those static variables. (I thought we already dropped those - whoops.) I think this drops this last of them, can you confirm David?
Regardless, this patch looks good to me.
Signed-off-by: David Gow davidgow@google.com
Reviewed-by: Brendan Higgins brendanhiggins@google.com
Thanks for taking care of this!
On Fri, Jan 24, 2020 at 2:01 PM Brendan Higgins brendanhiggins@google.com wrote:
Oh good, I am glad we are getting rid of those static variables. (I thought we already dropped those - whoops.) I think this drops this last of them, can you confirm David?
Yeah, this is the last of them.
I vaguely recall a suggestion that it may be worth testing that the LIST_HEAD() macro works with static, but as mentioned in the description, the for_each_entry_* tests probably aren't the best place to do that anyway...
-- David
On Fri, Jan 24, 2020 at 3:13 PM David Gow davidgow@google.com wrote:
On Fri, Jan 24, 2020 at 2:01 PM Brendan Higgins brendanhiggins@google.com wrote:
Oh good, I am glad we are getting rid of those static variables. (I thought we already dropped those - whoops.) I think this drops this last of them, can you confirm David?
Yeah, this is the last of them.
I vaguely recall a suggestion that it may be worth testing that the LIST_HEAD() macro works with static, but as mentioned in the description, the for_each_entry_* tests probably aren't the best place to do that anyway...
Ah, I think I missed that. Makes sense.
linux-kselftest-mirror@lists.linaro.org
-
Brendan Higgins -
David Gow