| Message ID | 20230830002116.3768675-1-davidgow@google.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | dce19a3fede254fd11e0013f1c6fe3bfc37a4d73 |
| Delegated to: | Brendan Higgins |
| Headers | show |
| Series | kunit: test: Make filter strings in executor_test writable | expand |
On Wed, Aug 30, 2023 at 08:21:15AM +0800, David Gow wrote: > KUnit's attribute filtering feature needs the filter strings passed in > to be writable, as it modifies them in-place during parsing. This works > for the filters passed on the kernel command line, but the string > literals used in the executor tests are at least theoretically read-only > (though they work on x86_64 for some reason). s390 wasn't fooled, and > crashed when these tests were run. > > Use a 'char[]' instead, (and make an explicit variable for the current > filter in parse_filter_attr_test), which will store the string in a > writable segment. > > Fixes: 76066f93f1df ("kunit: add tests for filtering attributes") > Closes: https://lore.kernel.org/linux-kselftest/55950256-c00a-4d21-a2c0-cf9f0e5b8a9a@roeck-us.net/ > Signed-off-by: David Gow <davidgow@google.com> Tested-by: Guenter Roeck <linux@roeck-us.net> Guenter > --- > lib/kunit/executor_test.c | 13 ++++++++----- > 1 file changed, 8 insertions(+), 5 deletions(-) > > diff --git a/lib/kunit/executor_test.c b/lib/kunit/executor_test.c > index 4084071d0eb5..b4f6f96b2844 100644 > --- a/lib/kunit/executor_test.c > +++ b/lib/kunit/executor_test.c > @@ -119,7 +119,7 @@ static void parse_filter_attr_test(struct kunit *test) > { > int j, filter_count; > struct kunit_attr_filter *parsed_filters; > - char *filters = "speed>slow, module!=example"; > + char filters[] = "speed>slow, module!=example", *filter = filters; > int err = 0; > > filter_count = kunit_get_filter_count(filters); > @@ -128,7 +128,7 @@ static void parse_filter_attr_test(struct kunit *test) > parsed_filters = kunit_kcalloc(test, filter_count, sizeof(*parsed_filters), > GFP_KERNEL); > for (j = 0; j < filter_count; j++) { > - parsed_filters[j] = kunit_next_attr_filter(&filters, &err); > + parsed_filters[j] = kunit_next_attr_filter(&filter, &err); > KUNIT_ASSERT_EQ_MSG(test, err, 0, "failed to parse filter '%s'", filters[j]); > } > > @@ -154,6 +154,7 @@ static void filter_attr_test(struct kunit *test) > .start = subsuite, .end = &subsuite[2], > }; > struct kunit_suite_set got; > + char filter[] = "speed>slow"; > int err = 0; > > subsuite[0] = alloc_fake_suite(test, "normal_suite", dummy_attr_test_cases); > @@ -168,7 +169,7 @@ static void filter_attr_test(struct kunit *test) > * attribute is unset and thus, the filtering is based on the parent attribute > * of slow. > */ > - got = kunit_filter_suites(&suite_set, NULL, "speed>slow", NULL, &err); > + got = kunit_filter_suites(&suite_set, NULL, filter, NULL, &err); > KUNIT_ASSERT_NOT_ERR_OR_NULL(test, got.start); > KUNIT_ASSERT_EQ(test, err, 0); > kfree_at_end(test, got.start); > @@ -191,12 +192,13 @@ static void filter_attr_empty_test(struct kunit *test) > .start = subsuite, .end = &subsuite[2], > }; > struct kunit_suite_set got; > + char filter[] = "module!=dummy"; > int err = 0; > > subsuite[0] = alloc_fake_suite(test, "suite1", dummy_attr_test_cases); > subsuite[1] = alloc_fake_suite(test, "suite2", dummy_attr_test_cases); > > - got = kunit_filter_suites(&suite_set, NULL, "module!=dummy", NULL, &err); > + got = kunit_filter_suites(&suite_set, NULL, filter, NULL, &err); > KUNIT_ASSERT_EQ(test, err, 0); > kfree_at_end(test, got.start); /* just in case */ > > @@ -211,12 +213,13 @@ static void filter_attr_skip_test(struct kunit *test) > .start = subsuite, .end = &subsuite[1], > }; > struct kunit_suite_set got; > + char filter[] = "speed>slow"; > int err = 0; > > subsuite[0] = alloc_fake_suite(test, "suite", dummy_attr_test_cases); > > /* Want: suite(slow, normal), NULL -> suite(slow with SKIP, normal), NULL */ > - got = kunit_filter_suites(&suite_set, NULL, "speed>slow", "skip", &err); > + got = kunit_filter_suites(&suite_set, NULL, filter, "skip", &err); > KUNIT_ASSERT_NOT_ERR_OR_NULL(test, got.start); > KUNIT_ASSERT_EQ(test, err, 0); > kfree_at_end(test, got.start); > -- > 2.42.0.283.g2d96d420d3-goog >
On Tue, Aug 29, 2023 at 8:21 PM David Gow <davidgow@google.com> wrote: > > KUnit's attribute filtering feature needs the filter strings passed in > to be writable, as it modifies them in-place during parsing. This works > for the filters passed on the kernel command line, but the string > literals used in the executor tests are at least theoretically read-only > (though they work on x86_64 for some reason). s390 wasn't fooled, and > crashed when these tests were run. > > Use a 'char[]' instead, (and make an explicit variable for the current > filter in parse_filter_attr_test), which will store the string in a > writable segment. > > Fixes: 76066f93f1df ("kunit: add tests for filtering attributes") > Closes: https://lore.kernel.org/linux-kselftest/55950256-c00a-4d21-a2c0-cf9f0e5b8a9a@roeck-us.net/ > Signed-off-by: David Gow <davidgow@google.com> Hello! This looks good to me. Thanks for doing this! Reviewed-by: Rae Moar <rmoar@google.com> -Rae > --- > lib/kunit/executor_test.c | 13 ++++++++----- > 1 file changed, 8 insertions(+), 5 deletions(-) > > diff --git a/lib/kunit/executor_test.c b/lib/kunit/executor_test.c > index 4084071d0eb5..b4f6f96b2844 100644 > --- a/lib/kunit/executor_test.c > +++ b/lib/kunit/executor_test.c > @@ -119,7 +119,7 @@ static void parse_filter_attr_test(struct kunit *test) > { > int j, filter_count; > struct kunit_attr_filter *parsed_filters; > - char *filters = "speed>slow, module!=example"; > + char filters[] = "speed>slow, module!=example", *filter = filters; > int err = 0; > > filter_count = kunit_get_filter_count(filters); > @@ -128,7 +128,7 @@ static void parse_filter_attr_test(struct kunit *test) > parsed_filters = kunit_kcalloc(test, filter_count, sizeof(*parsed_filters), > GFP_KERNEL); > for (j = 0; j < filter_count; j++) { > - parsed_filters[j] = kunit_next_attr_filter(&filters, &err); > + parsed_filters[j] = kunit_next_attr_filter(&filter, &err); > KUNIT_ASSERT_EQ_MSG(test, err, 0, "failed to parse filter '%s'", filters[j]); > } > > @@ -154,6 +154,7 @@ static void filter_attr_test(struct kunit *test) > .start = subsuite, .end = &subsuite[2], > }; > struct kunit_suite_set got; > + char filter[] = "speed>slow"; > int err = 0; > > subsuite[0] = alloc_fake_suite(test, "normal_suite", dummy_attr_test_cases); > @@ -168,7 +169,7 @@ static void filter_attr_test(struct kunit *test) > * attribute is unset and thus, the filtering is based on the parent attribute > * of slow. > */ > - got = kunit_filter_suites(&suite_set, NULL, "speed>slow", NULL, &err); > + got = kunit_filter_suites(&suite_set, NULL, filter, NULL, &err); > KUNIT_ASSERT_NOT_ERR_OR_NULL(test, got.start); > KUNIT_ASSERT_EQ(test, err, 0); > kfree_at_end(test, got.start); > @@ -191,12 +192,13 @@ static void filter_attr_empty_test(struct kunit *test) > .start = subsuite, .end = &subsuite[2], > }; > struct kunit_suite_set got; > + char filter[] = "module!=dummy"; > int err = 0; > > subsuite[0] = alloc_fake_suite(test, "suite1", dummy_attr_test_cases); > subsuite[1] = alloc_fake_suite(test, "suite2", dummy_attr_test_cases); > > - got = kunit_filter_suites(&suite_set, NULL, "module!=dummy", NULL, &err); > + got = kunit_filter_suites(&suite_set, NULL, filter, NULL, &err); > KUNIT_ASSERT_EQ(test, err, 0); > kfree_at_end(test, got.start); /* just in case */ > > @@ -211,12 +213,13 @@ static void filter_attr_skip_test(struct kunit *test) > .start = subsuite, .end = &subsuite[1], > }; > struct kunit_suite_set got; > + char filter[] = "speed>slow"; > int err = 0; > > subsuite[0] = alloc_fake_suite(test, "suite", dummy_attr_test_cases); > > /* Want: suite(slow, normal), NULL -> suite(slow with SKIP, normal), NULL */ > - got = kunit_filter_suites(&suite_set, NULL, "speed>slow", "skip", &err); > + got = kunit_filter_suites(&suite_set, NULL, filter, "skip", &err); > KUNIT_ASSERT_NOT_ERR_OR_NULL(test, got.start); > KUNIT_ASSERT_EQ(test, err, 0); > kfree_at_end(test, got.start); > -- > 2.42.0.283.g2d96d420d3-goog >
diff --git a/lib/kunit/executor_test.c b/lib/kunit/executor_test.c index 4084071d0eb5..b4f6f96b2844 100644 --- a/lib/kunit/executor_test.c +++ b/lib/kunit/executor_test.c @@ -119,7 +119,7 @@ static void parse_filter_attr_test(struct kunit *test) { int j, filter_count; struct kunit_attr_filter *parsed_filters; - char *filters = "speed>slow, module!=example"; + char filters[] = "speed>slow, module!=example", *filter = filters; int err = 0; filter_count = kunit_get_filter_count(filters); @@ -128,7 +128,7 @@ static void parse_filter_attr_test(struct kunit *test) parsed_filters = kunit_kcalloc(test, filter_count, sizeof(*parsed_filters), GFP_KERNEL); for (j = 0; j < filter_count; j++) { - parsed_filters[j] = kunit_next_attr_filter(&filters, &err); + parsed_filters[j] = kunit_next_attr_filter(&filter, &err); KUNIT_ASSERT_EQ_MSG(test, err, 0, "failed to parse filter '%s'", filters[j]); } @@ -154,6 +154,7 @@ static void filter_attr_test(struct kunit *test) .start = subsuite, .end = &subsuite[2], }; struct kunit_suite_set got; + char filter[] = "speed>slow"; int err = 0; subsuite[0] = alloc_fake_suite(test, "normal_suite", dummy_attr_test_cases); @@ -168,7 +169,7 @@ static void filter_attr_test(struct kunit *test) * attribute is unset and thus, the filtering is based on the parent attribute * of slow. */ - got = kunit_filter_suites(&suite_set, NULL, "speed>slow", NULL, &err); + got = kunit_filter_suites(&suite_set, NULL, filter, NULL, &err); KUNIT_ASSERT_NOT_ERR_OR_NULL(test, got.start); KUNIT_ASSERT_EQ(test, err, 0); kfree_at_end(test, got.start); @@ -191,12 +192,13 @@ static void filter_attr_empty_test(struct kunit *test) .start = subsuite, .end = &subsuite[2], }; struct kunit_suite_set got; + char filter[] = "module!=dummy"; int err = 0; subsuite[0] = alloc_fake_suite(test, "suite1", dummy_attr_test_cases); subsuite[1] = alloc_fake_suite(test, "suite2", dummy_attr_test_cases); - got = kunit_filter_suites(&suite_set, NULL, "module!=dummy", NULL, &err); + got = kunit_filter_suites(&suite_set, NULL, filter, NULL, &err); KUNIT_ASSERT_EQ(test, err, 0); kfree_at_end(test, got.start); /* just in case */ @@ -211,12 +213,13 @@ static void filter_attr_skip_test(struct kunit *test) .start = subsuite, .end = &subsuite[1], }; struct kunit_suite_set got; + char filter[] = "speed>slow"; int err = 0; subsuite[0] = alloc_fake_suite(test, "suite", dummy_attr_test_cases); /* Want: suite(slow, normal), NULL -> suite(slow with SKIP, normal), NULL */ - got = kunit_filter_suites(&suite_set, NULL, "speed>slow", "skip", &err); + got = kunit_filter_suites(&suite_set, NULL, filter, "skip", &err); KUNIT_ASSERT_NOT_ERR_OR_NULL(test, got.start); KUNIT_ASSERT_EQ(test, err, 0); kfree_at_end(test, got.start);
KUnit's attribute filtering feature needs the filter strings passed in to be writable, as it modifies them in-place during parsing. This works for the filters passed on the kernel command line, but the string literals used in the executor tests are at least theoretically read-only (though they work on x86_64 for some reason). s390 wasn't fooled, and crashed when these tests were run. Use a 'char[]' instead, (and make an explicit variable for the current filter in parse_filter_attr_test), which will store the string in a writable segment. Fixes: 76066f93f1df ("kunit: add tests for filtering attributes") Closes: https://lore.kernel.org/linux-kselftest/55950256-c00a-4d21-a2c0-cf9f0e5b8a9a@roeck-us.net/ Signed-off-by: David Gow <davidgow@google.com> --- lib/kunit/executor_test.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)