Re: [PATCH i-g-t v3 5/6] lib/igt_ktap: Just free ktap

[Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>]

On Thursday, 7 November 2024 06:52:53 CET Lucas De Marchi wrote:
> Change function signature so it only set the pointer free rather than
> also setting it to NULL. This matches alloc/free behavior for other
> objects in igt.
> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>

Reviewed-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>

> ---
>  lib/igt_kmod.c              |  2 +-
>  lib/igt_ktap.c              |  5 ++---
>  lib/igt_ktap.h              |  2 +-
>  lib/tests/igt_ktap_parser.c | 24 ++++++++++++------------
>  4 files changed, 16 insertions(+), 17 deletions(-)
> 
> diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
> index fd79e8739..7be95a61c 100644
> --- a/lib/igt_kmod.c
> +++ b/lib/igt_kmod.c
> @@ -1026,7 +1026,7 @@ static int kunit_get_results(struct igt_list_head *results, const char *debugfs_
>  	}
>  
>  	free(buf);
> -	igt_ktap_free(&ktap);
> +	igt_ktap_free(ktap);
>  
>  out_fclose:
>  	fclose(results_stream);
> diff --git a/lib/igt_ktap.c b/lib/igt_ktap.c
> index 300fb2bb5..aa7ea8447 100644
> --- a/lib/igt_ktap.c
> +++ b/lib/igt_ktap.c
> @@ -310,8 +310,7 @@ 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);
> -	*ktap = NULL;
> +	free(ktap);
>  }
> diff --git a/lib/igt_ktap.h b/lib/igt_ktap.h
> index 7684e859b..c422636bf 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 8c2d16080..6357bdf6a 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
>