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@xxxxxxxxxx> 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@xxxxxxxxxx>
>
> 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
