All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Fix linked-list KUnit test when run multiple times
@ 2020-01-24 19:45 David Gow
  2020-01-24 22:01 ` Brendan Higgins
  0 siblings, 1 reply; 4+ messages in thread
From: David Gow @ 2020-01-24 19:45 UTC (permalink / raw)
  To: brendanhiggins, shuah
  Cc: kunit-dev, linux-kselftest, linux-kernel, alan.maguire, David Gow

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) {
-- 
2.25.0.341.g760bfbb309-goog


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] Fix linked-list KUnit test when run multiple times
  2020-01-24 19:45 [PATCH] Fix linked-list KUnit test when run multiple times David Gow
@ 2020-01-24 22:01 ` Brendan Higgins
  2020-01-24 23:13   ` David Gow
  0 siblings, 1 reply; 4+ messages in thread
From: Brendan Higgins @ 2020-01-24 22:01 UTC (permalink / raw)
  To: David Gow
  Cc: shuah, KUnit Development, open list:KERNEL SELFTEST FRAMEWORK,
	Linux Kernel Mailing List, Alan Maguire

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!

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Fix linked-list KUnit test when run multiple times
  2020-01-24 22:01 ` Brendan Higgins
@ 2020-01-24 23:13   ` David Gow
  2020-01-25  0:02     ` Brendan Higgins
  0 siblings, 1 reply; 4+ messages in thread
From: David Gow @ 2020-01-24 23:13 UTC (permalink / raw)
  To: Brendan Higgins
  Cc: shuah, KUnit Development, open list:KERNEL SELFTEST FRAMEWORK,
	Linux Kernel Mailing List, Alan Maguire

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Fix linked-list KUnit test when run multiple times
  2020-01-24 23:13   ` David Gow
@ 2020-01-25  0:02     ` Brendan Higgins
  0 siblings, 0 replies; 4+ messages in thread
From: Brendan Higgins @ 2020-01-25  0:02 UTC (permalink / raw)
  To: David Gow
  Cc: shuah, KUnit Development, open list:KERNEL SELFTEST FRAMEWORK,
	Linux Kernel Mailing List, Alan Maguire

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.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2020-01-25  0:02 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-24 19:45 [PATCH] Fix linked-list KUnit test when run multiple times David Gow
2020-01-24 22:01 ` Brendan Higgins
2020-01-24 23:13   ` David Gow
2020-01-25  0:02     ` Brendan Higgins

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.