On Tue, Oct 29, 2019 at 6:00 AM shuah <shuah@xxxxxxxxxx> wrote: > On 10/24/19 4:46 PM, David Gow wrote: > > Add a KUnit test for the kernel doubly linked list implementation in > > include/linux/list.h > > > > Each test case (list_test_x) is focused on testing the behaviour of the > > list function/macro 'x'. None of the tests pass invalid lists to these > > macros, and so should behave identically with DEBUG_LIST enabled and > > disabled. > > > > Note that, at present, it only tests the list_ types (not the > > singly-linked hlist_), and does not yet test all of the > > list_for_each_entry* macros (and some related things like > > list_prepare_entry). > > > > Signed-off-by: David Gow <davidgow@xxxxxxxxxx> > > Reviewed-by: Brendan Higgins <brendanhiggins@xxxxxxxxxx> > > Tested-by: Brendan Higgins <brendanhiggins@xxxxxxxxxx> > > --- > > > > This revision addresses Brendan's comments in > > https://lore.kernel.org/linux-kselftest/20191023220248.GA55483@xxxxxxxxxx/ > > > > Specifically: > > - Brendan's Reviewed-by/Tested-by being included in the description. > > - A couple of trailing tabs in Kconfig.debug & list-test.c > > - Reformatting of previously >80 character lines. > > > > > > Earlier versions of this patchset can be found: > > > > v5: > > https://lore.kernel.org/linux-kselftest/20191022221322.122788-1-davidgow@xxxxxxxxxx/ > > v4: > > https://lore.kernel.org/linux-kselftest/20191018215549.65000-1-davidgow@xxxxxxxxxx/ > > v3: > > https://lore.kernel.org/linux-kselftest/20191016215707.95317-1-davidgow@xxxxxxxxxx/ > > v2: > > https://lore.kernel.org/linux-kselftest/20191010185631.26541-1-davidgow@xxxxxxxxxx/ > > v1: > > https://lore.kernel.org/linux-kselftest/20191007213633.92565-1-davidgow@xxxxxxxxxx/ > > > > CHECK: Unnecessary parentheses around test_struct.list > #699: FILE: lib/list-test.c:510: > + KUNIT_EXPECT_PTR_EQ(test, &test_struct, list_entry(&(test_struct.list), > > CHECK: Alignment should match open parenthesis > #700: FILE: lib/list-test.c:511: > + KUNIT_EXPECT_PTR_EQ(test, &test_struct, list_entry(&(test_struct.list), > + struct list_test_struct, list)); > > CHECK: Please don't use multiple blank lines > #711: FILE: lib/list-test.c:522: > + > + > > CHECK: Alignment should match open parenthesis > #713: FILE: lib/list-test.c:524: > + KUNIT_EXPECT_PTR_EQ(test, &test_struct1, list_first_entry(&list, > + struct list_test_struct, list)); > > CHECK: Please don't use multiple blank lines > #724: FILE: lib/list-test.c:535: > + > + > > CHECK: Alignment should match open parenthesis > #726: FILE: lib/list-test.c:537: > + KUNIT_EXPECT_PTR_EQ(test, &test_struct2, list_last_entry(&list, > + struct list_test_struct, list)); > > CHECK: Alignment should match open parenthesis > #735: FILE: lib/list-test.c:546: > + KUNIT_EXPECT_FALSE(test, list_first_entry_or_null(&list, > + struct list_test_struct, list)); > > CHECK: Alignment should match open parenthesis > #741: FILE: lib/list-test.c:552: > + KUNIT_EXPECT_PTR_EQ(test, &test_struct1, > + list_first_entry_or_null(&list, > > CHECK: Alignment should match open parenthesis > #742: FILE: lib/list-test.c:553: > + list_first_entry_or_null(&list, > + struct list_test_struct, list)); > > CHECK: Please don't use multiple blank lines > #753: FILE: lib/list-test.c:564: > + > + > > CHECK: Alignment should match open parenthesis > #755: FILE: lib/list-test.c:566: > + KUNIT_EXPECT_PTR_EQ(test, &test_struct2, list_next_entry(&test_struct1, > + list)); > > CHECK: Please don't use multiple blank lines > #766: FILE: lib/list-test.c:577: > + > + > > CHECK: Alignment should match open parenthesis > #768: FILE: lib/list-test.c:579: > + KUNIT_EXPECT_PTR_EQ(test, &test_struct1, list_prev_entry(&test_struct2, > + list)); > > ERROR: that open brace { should be on the previous line > #789: FILE: lib/list-test.c:600: > +static void list_test_list_for_each_prev(struct kunit *test) > +{ > > ERROR: that open brace { should be on the previous line > #807: FILE: lib/list-test.c:618: > +static void list_test_list_for_each_safe(struct kunit *test) > +{ > > CHECK: Please don't use multiple blank lines > #813: FILE: lib/list-test.c:624: > + > + > > ERROR: that open brace { should be on the previous line > #828: FILE: lib/list-test.c:639: > +static void list_test_list_for_each_prev_safe(struct kunit *test) > +{ > > ERROR: that open brace { should be on the previous line > #848: FILE: lib/list-test.c:659: > +static void list_test_list_for_each_entry(struct kunit *test) > +{ > > ERROR: that open brace { should be on the previous line > #869: FILE: lib/list-test.c:680: > +static void list_test_list_for_each_entry_reverse(struct kunit *test) > +{ > > > I am seeing these error and warns. As per our hallway conversation, the > "for_each*" in the test naming is tripping up checkpatch.pl > > For now you can change the name a bit to not trip checkpatch and maybe > explore fixing checkpatch to differentiate between function names > with "for_each" in them vs. the actual for_each usages in the code. Thanks, Shuah. Yes, the problem here is that checkpatch.pl believes that anything with "for_each" in its name must be a loop, so expects that the open brace is placed on the same line as for a for loop. Longer term, I think it'd be nicer, naming-wise, to fix or work around this issue in checkpatch.pl itself, as that'd allow the tests to continue to follow a naming pattern of "list_test_[x]", where [x] is the name of the function/macro being tested. Of course, short of trying to fit a whole C parser in checkpatch.pl, that's going to involve some compromises as well. In the meantime, I'm sending out v7 which replaces "for_each" with "for__each" (adding the extra underscore), so that checkpatch is happy. Cheers, -- David
