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(-)
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);
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.
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.
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;
Similar to above I think this should be changed to a KUNIT_ASSERT statement to ensure the result is equal to 0.
KUNIT_EXPECT_STREQ(test, filter.suite_glob, "suite"); KUNIT_EXPECT_STREQ(test, filter.test_glob, "test"); kfree(filter.suite_glob);
-- 2.39.2
-- You received this message because you are subscribed to the Google Groups "KUnit Development" group. To unsubscribe from this group and stop receiving emails from it, send an email to kunit-dev+unsubscribe@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/3ff9d019-75b6-45ee-9b03-9d6ec7c5....
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
linux-kselftest-mirror@lists.linaro.org