Hi Janusz, On 2024-03-18 at 11:13:28 +0100, Janusz Krzysztofik wrote: > Users who store a pointer to struct igt_ktap_results, obtained from > igt_ktap_alloc(), in a central location and then call igt_ktap_free() when > no longer needed, now have to reset that pointer to NULL to avoid double > free on final cleanup. For their convenience, teach igt_ktap_free() to > accept that location as an argument and reset the pointer after freeing > the structure. > > Signed-off-by: Janusz Krzysztofik <[email protected]>
LGTM, Reviewed-by: Kamil Konieczny <[email protected]> > --- > lib/igt_kmod.c | 12 +++++------- > lib/igt_ktap.c | 5 +++-- > lib/igt_ktap.h | 2 +- > lib/tests/igt_ktap_parser.c | 24 ++++++++++++------------ > 4 files changed, 21 insertions(+), 22 deletions(-) > > diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c > index ca20012a97..8a6824ea7e 100644 > --- a/lib/igt_kmod.c > +++ b/lib/igt_kmod.c > @@ -1094,7 +1094,7 @@ static void __igt_kunit_legacy(struct igt_ktest *tst, > > if (igt_debug_on(pthread_create(&modprobe.thread, NULL, > modprobe_task, &modprobe))) { > - igt_ktap_free(ktap); > + igt_ktap_free(&ktap); > igt_skip("Failed to create a modprobe thread\n"); > } > > @@ -1197,7 +1197,7 @@ static void __igt_kunit_legacy(struct igt_ktest *tst, > break; > } > > - igt_ktap_free(ktap); > + igt_ktap_free(&ktap); > > igt_skip_on(modprobe.err); > igt_skip_on(igt_kernel_tainted(&taints)); > @@ -1243,8 +1243,7 @@ static bool kunit_get_tests(struct igt_list_head *tests, > err = kunit_kmsg_result_get(tests, NULL, tst->kmsg, *ktap); > while (err == -EINPROGRESS); > > - igt_ktap_free(*ktap); > - *ktap = NULL; > + igt_ktap_free(ktap); > > igt_skip_on_f(err, > "KTAP parser failed while getting a list of test > cases\n"); > @@ -1405,8 +1404,7 @@ static void __igt_kunit(struct igt_ktest *tst, > } > } > > - igt_ktap_free(*ktap); > - *ktap = NULL; > + igt_ktap_free(ktap); > > igt_skip_on(modprobe.err); > igt_skip_on(igt_kernel_tainted(&taints)); > @@ -1487,7 +1485,7 @@ void igt_kunit(const char *module_name, const char > *suite, const char *opts) > igt_fixture { > char *suite_name = NULL, *case_name = NULL; > > - igt_ktap_free(ktap); > + igt_ktap_free(&ktap); > > kunit_results_free(&tests, &suite_name, &case_name); > > diff --git a/lib/igt_ktap.c b/lib/igt_ktap.c > index aa7ea84476..300fb2bb5a 100644 > --- a/lib/igt_ktap.c > +++ b/lib/igt_ktap.c > @@ -310,7 +310,8 @@ struct igt_ktap_results *igt_ktap_alloc(struct > igt_list_head *results) > return ktap; > } > > -void igt_ktap_free(struct igt_ktap_results *ktap) > +void igt_ktap_free(struct igt_ktap_results **ktap) > { > - free(ktap); > + free(*ktap); > + *ktap = NULL; > } > diff --git a/lib/igt_ktap.h b/lib/igt_ktap.h > index c422636bfc..7684e859b3 100644 > --- a/lib/igt_ktap.h > +++ b/lib/igt_ktap.h > @@ -41,6 +41,6 @@ struct igt_ktap_results; > > struct igt_ktap_results *igt_ktap_alloc(struct igt_list_head *results); > int igt_ktap_parse(const char *buf, struct igt_ktap_results *ktap); > -void igt_ktap_free(struct igt_ktap_results *ktap); > +void igt_ktap_free(struct igt_ktap_results **ktap); > > #endif /* IGT_KTAP_H */ > diff --git a/lib/tests/igt_ktap_parser.c b/lib/tests/igt_ktap_parser.c > index 6357bdf6a5..8c2d16080d 100644 > --- a/lib/tests/igt_ktap_parser.c > +++ b/lib/tests/igt_ktap_parser.c > @@ -45,7 +45,7 @@ static void ktap_list(void) > igt_assert_eq(igt_ktap_parse(" ok 4 test_case_4 # SKIP\n", ktap), > -EINPROGRESS); > igt_assert_eq(igt_ktap_parse("ok 3 test_suite_3\n", ktap), 0); > > - igt_ktap_free(ktap); > + igt_ktap_free(&ktap); > > igt_assert_eq(igt_list_length(&results), 8); > > @@ -107,7 +107,7 @@ static void ktap_results(void) > igt_assert_eq(igt_ktap_parse(" ok 1 test_case\n", ktap), > -EINPROGRESS); > igt_assert_eq(igt_ktap_parse("not ok 1 test_suite\n", ktap), 0); > > - igt_ktap_free(ktap); > + igt_ktap_free(&ktap); > > igt_assert_eq(igt_list_length(&results), 2); > > @@ -162,7 +162,7 @@ static void ktap_success(void) > igt_assert_eq(igt_ktap_parse("not ok 1 test_suite\n", ktap), 0); > igt_assert_eq(igt_list_length(&results), 2); > > - igt_ktap_free(ktap); > + igt_ktap_free(&ktap); > > result = igt_list_last_entry(&results, result, link); > igt_list_del(&result->link); > @@ -186,48 +186,48 @@ static void ktap_top_version(void) > ktap = igt_ktap_alloc(&results); > igt_require(ktap); > igt_assert_eq(igt_ktap_parse("1..1\n", ktap), -EPROTO); > - igt_ktap_free(ktap); > + igt_ktap_free(&ktap); > > ktap = igt_ktap_alloc(&results); > igt_require(ktap); > /* TODO: change to -EPROTO as soon as related workaround is dropped */ > igt_assert_eq(igt_ktap_parse(" KTAP version 1\n", ktap), > -EINPROGRESS); > - igt_ktap_free(ktap); > + igt_ktap_free(&ktap); > > ktap = igt_ktap_alloc(&results); > igt_require(ktap); > igt_assert_eq(igt_ktap_parse(" # Subtest: test_suite\n", ktap), > -EPROTO); > - igt_ktap_free(ktap); > + igt_ktap_free(&ktap); > > ktap = igt_ktap_alloc(&results); > igt_require(ktap); > igt_assert_eq(igt_ktap_parse(" 1..1\n", ktap), -EPROTO); > - igt_ktap_free(ktap); > + igt_ktap_free(&ktap); > > ktap = igt_ktap_alloc(&results); > igt_require(ktap); > igt_assert_eq(igt_ktap_parse(" KTAP version 1\n", ktap), > -EPROTO); > - igt_ktap_free(ktap); > + igt_ktap_free(&ktap); > > ktap = igt_ktap_alloc(&results); > igt_require(ktap); > igt_assert_eq(igt_ktap_parse(" # Subtest: test_case\n", ktap), > -EPROTO); > - igt_ktap_free(ktap); > + igt_ktap_free(&ktap); > > ktap = igt_ktap_alloc(&results); > igt_require(ktap); > igt_assert_eq(igt_ktap_parse(" ok 1 parameter 1\n", ktap), > -EPROTO); > - igt_ktap_free(ktap); > + igt_ktap_free(&ktap); > > ktap = igt_ktap_alloc(&results); > igt_require(ktap); > igt_assert_eq(igt_ktap_parse(" ok 1 test_case\n", ktap), -EPROTO); > - igt_ktap_free(ktap); > + igt_ktap_free(&ktap); > > ktap = igt_ktap_alloc(&results); > igt_require(ktap); > igt_assert_eq(igt_ktap_parse("ok 1 test_suite\n", ktap), -EPROTO); > - igt_ktap_free(ktap); > + igt_ktap_free(&ktap); > } > > igt_main > -- > 2.43.0 >
