| Message ID | 3ff9d019-75b6-45ee-9b03-9d6ec7c5364f@moroto.mountain |
|---|---|
| State | New |
| Headers | show |
| Series | kunit: Check for kunit_parse_glob_filter() failure | expand |
On Tue, Sep 19, 2023 at 02:49:43PM -0400, Rae Moar wrote: > On Fri, Sep 15, 2023 at 8:58 AM Dan Carpenter <dan.carpenter@linaro.org> wrote: > > > > Smatch complains that the missing error checks would lead to a crash: > > > > lib/kunit/executor_test.c:40 parse_filter_test() > > error: double free of 'filter.test_glob' > > > > We may as well do it right... > > > > Fixes: a127b154a8f2 ("kunit: tool: allow filtering test cases via glob") > > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> > > Hello! > > We definitely should add in checks for these errors. I have a couple > comments below. > > Thanks! > -Rae > > > --- > > lib/kunit/executor_test.c | 6 ++++-- > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/lib/kunit/executor_test.c b/lib/kunit/executor_test.c > > index b4f6f96b2844..176c9c9dfcfc 100644 > > --- a/lib/kunit/executor_test.c > > +++ b/lib/kunit/executor_test.c > > @@ -27,13 +27,15 @@ static void parse_filter_test(struct kunit *test) > > { > > struct kunit_glob_filter filter = {NULL, NULL}; > > > > - kunit_parse_glob_filter(&filter, "suite"); > > + if (!kunit_parse_glob_filter(&filter, "suite")) > > + return; > > First, this is returning every time this test is run because the > kunit_parse_glob_filter returns 0 when there is no error. So this > should instead be checking for a result of not equal to 0. Oh... Duh. Sorry. That's embarrassing. > > Secondly, this should fail the test if the parsing returns an error. > So instead of returning I would recommend using a KUNIT_ASSERT > statement to check the result of this method is equal to 0. Will do. Thanks! regards, dan carpenter
diff --git a/lib/kunit/executor_test.c b/lib/kunit/executor_test.c index b4f6f96b2844..176c9c9dfcfc 100644 --- a/lib/kunit/executor_test.c +++ b/lib/kunit/executor_test.c @@ -27,13 +27,15 @@ static void parse_filter_test(struct kunit *test) { struct kunit_glob_filter filter = {NULL, NULL}; - kunit_parse_glob_filter(&filter, "suite"); + if (!kunit_parse_glob_filter(&filter, "suite")) + return; KUNIT_EXPECT_STREQ(test, filter.suite_glob, "suite"); KUNIT_EXPECT_FALSE(test, filter.test_glob); kfree(filter.suite_glob); kfree(filter.test_glob); - kunit_parse_glob_filter(&filter, "suite.test"); + if (!kunit_parse_glob_filter(&filter, "suite.test")) + return; KUNIT_EXPECT_STREQ(test, filter.suite_glob, "suite"); KUNIT_EXPECT_STREQ(test, filter.test_glob, "test"); kfree(filter.suite_glob);
Smatch complains that the missing error checks would lead to a crash: lib/kunit/executor_test.c:40 parse_filter_test() error: double free of 'filter.test_glob' We may as well do it right... Fixes: a127b154a8f2 ("kunit: tool: allow filtering test cases via glob") Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> --- lib/kunit/executor_test.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)