From: Kamil Konieczny <kamil.konieczny@linux.intel.com>
To: igt-dev@lists.freedesktop.org
Cc: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>,
intel-gfx@lists.freedesktop.org, intel-xe@lists.freedesktop.org,
Mauro Carvalho Chehab <mchehab@kernel.org>,
Jonathan Cavitt <jonathan.cavitt@intel.com>,
Lucas De Marchi <lucas.demarchi@intel.com>
Subject: Re: [PATCH i-g-t v3 2/5] lib/kunit: Let igt_ktap_free() take care of pointer reset
Date: Tue, 19 Mar 2024 19:30:40 +0100 [thread overview]
Message-ID: <20240319183040.xljhpgtursfgltc4@kamilkon-desk.igk.intel.com> (raw)
In-Reply-To: <20240318103534.701693-9-janusz.krzysztofik@linux.intel.com>
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 <janusz.krzysztofik@linux.intel.com>
LGTM,
Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
> ---
> 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
>
next prev parent reply other threads:[~2024-03-19 18:30 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-18 10:13 [PATCH i-g-t v3 0/5] lib/kunit: Execute test cases synchronously Janusz Krzysztofik
2024-03-18 10:13 ` [PATCH i-g-t v3 1/5] lib/kunit: Store igt_ktap_results pointer in a central location Janusz Krzysztofik
2024-03-19 18:25 ` Kamil Konieczny
2024-03-18 10:13 ` [PATCH i-g-t v3 2/5] lib/kunit: Let igt_ktap_free() take care of pointer reset Janusz Krzysztofik
2024-03-19 18:30 ` Kamil Konieczny [this message]
2024-03-18 10:13 ` [PATCH i-g-t v3 3/5] lib/kunit: Time out promptly on missing KTAP report Janusz Krzysztofik
2024-03-19 18:33 ` Kamil Konieczny
2024-03-18 10:13 ` [PATCH i-g-t v3 4/5] lib/kunit: Execute test cases synchronously Janusz Krzysztofik
2024-03-25 17:04 ` Kamil Konieczny
2024-03-18 10:13 ` [PATCH i-g-t v3 5/5] lib/kunit: Minimize code duplication Janusz Krzysztofik
2024-03-25 17:12 ` Kamil Konieczny
2024-03-18 17:21 ` ✗ CI.Patch_applied: failure for lib/kunit: Execute test cases synchronously (rev3) Patchwork
2024-03-18 17:59 ` ✓ CI.xeBAT: success for lib/kunit: Execute test cases synchronously (rev6) Patchwork
2024-03-18 18:10 ` ✓ Fi.CI.BAT: " Patchwork
2024-03-18 23:28 ` ✗ Fi.CI.IGT: failure " Patchwork
2024-03-19 18:39 ` [PATCH i-g-t v3 0/5] lib/kunit: Execute test cases synchronously Cavitt, Jonathan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240319183040.xljhpgtursfgltc4@kamilkon-desk.igk.intel.com \
--to=kamil.konieczny@linux.intel.com \
--cc=igt-dev@lists.freedesktop.org \
--cc=intel-gfx@lists.freedesktop.org \
--cc=intel-xe@lists.freedesktop.org \
--cc=janusz.krzysztofik@linux.intel.com \
--cc=jonathan.cavitt@intel.com \
--cc=lucas.demarchi@intel.com \
--cc=mchehab@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.