diff mbox series

[i-g-t,v3,2/5] lib/kunit: Let igt_ktap_free() take care of pointer reset

Message ID 20240318103534.701693-9-janusz.krzysztofik@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series lib/kunit: Execute test cases synchronously | expand

Commit Message

Janusz Krzysztofik March 18, 2024, 10:13 a.m. UTC 
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>
---
 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(-)

Comments

Kamil Konieczny March 19, 2024, 6:30 p.m. UTC | #1 
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
>
diff mbox series

Patch

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