Re: [PATCH i-g-t v3 4/6] lib/igt_kmod: Stop passing ktap around

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

On Thursday, 7 November 2024 06:52:52 CET Lucas De Marchi wrote:
> Just allocate and free in the only function it's needed.
> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>

Looks safe.

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

> ---
>  lib/igt_kmod.c | 29 ++++++++++++-----------------
>  1 file changed, 12 insertions(+), 17 deletions(-)
> 
> diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
> index 36578aa68..fd79e8739 100644
> --- a/lib/igt_kmod.c
> +++ b/lib/igt_kmod.c
> @@ -995,8 +995,9 @@ static void kunit_results_free(struct igt_list_head *results,
>  }
>  
>  static int kunit_get_results(struct igt_list_head *results, const char *debugfs_path,
> -			     const char *suite, struct igt_ktap_results **ktap)
> +			     const char *suite)
>  {
> +	struct igt_ktap_results *ktap;
>  	char results_path[PATH_MAX];
>  	FILE *results_stream;
>  	char *buf = NULL;
> @@ -1012,21 +1013,21 @@ static int kunit_get_results(struct igt_list_head *results, const char *debugfs_
>  	if (igt_debug_on(!results_stream))
>  		return -errno;
>  
> -	*ktap = igt_ktap_alloc(results);
> -	if (igt_debug_on(!*ktap)) {
> +	ktap = igt_ktap_alloc(results);
> +	if (igt_debug_on(!ktap)) {
>  		err = -ENOMEM;
>  		goto out_fclose;
>  	}
>  
>  	while (len = getline(&buf, &size, results_stream), len > 0) {
> -		err = igt_ktap_parse(buf, *ktap);
> +		err = igt_ktap_parse(buf, ktap);
>  		if (err != -EINPROGRESS)
>  			break;
>  	}
>  
>  	free(buf);
> +	igt_ktap_free(&ktap);
>  
> -	igt_ktap_free(ktap);
>  out_fclose:
>  	fclose(results_stream);
>  
> @@ -1037,8 +1038,7 @@ static void kunit_get_tests(struct igt_list_head *tests,
>  			    struct igt_ktest *tst,
>  			    const char *suite,
>  			    const char *opts,
> -			    const char *debugfs_path,
> -			    struct igt_ktap_results **ktap)
> +			    const char *debugfs_path)
>  {
>  	struct igt_ktap_result *r, *rn;
>  	struct dirent *subdir;
> @@ -1083,7 +1083,7 @@ static void kunit_get_tests(struct igt_list_head *tests,
>  		if (suite && strcmp(subdir->d_name, suite))
>  			continue;
>  
> -		igt_warn_on_f(kunit_get_results(tests, debugfs_path, subdir->d_name, ktap),
> +		igt_warn_on_f(kunit_get_results(tests, debugfs_path, subdir->d_name),
>  			      "parsing KTAP report from test suite \"%s\" failed\n",
>  			      subdir->d_name);
>  
> @@ -1102,8 +1102,7 @@ static void __igt_kunit(struct igt_ktest *tst,
>  			const char *subtest,
>  			const char *opts,
>  			const char *debugfs_path,
> -			struct igt_list_head *tests,
> -			struct igt_ktap_results **ktap)
> +			struct igt_list_head *tests)
>  {
>  	struct igt_ktap_result *t;
>  
> @@ -1133,7 +1132,7 @@ static void __igt_kunit(struct igt_ktest *tst,
>  			igt_assert_eq(igt_kernel_tainted(&taints), 0);
>  
>  			igt_assert_eq(kunit_get_results(&results, debugfs_path,
> -							t->suite_name, ktap), 0);
> +							t->suite_name), 0);
>  
>  			for (i = 0; i < 2; i++) {
>  				kunit_result_free(&r, &suite_name, &case_name);
> @@ -1197,7 +1196,6 @@ void igt_kunit(const char *module_name, const char *suite, const char *opts)
>  {
>  	char debugfs_path[PATH_MAX] = { '\0', };
>  	struct igt_ktest tst = { .kmsg = -1, };
> -	struct igt_ktap_results *ktap = NULL;
>  	char *subtest;
>  	IGT_LIST_HEAD(tests);
>  
> @@ -1243,16 +1241,13 @@ void igt_kunit(const char *module_name, const char *suite, const char *opts)
>  	 * and for documentation.
>  	 */
>  	igt_subtest_with_dynamic(subtest) {
> -		kunit_get_tests(&tests, &tst, suite, opts,
> -				debugfs_path, &ktap);
> -		__igt_kunit(&tst, subtest, opts, debugfs_path, &tests, &ktap);
> +		kunit_get_tests(&tests, &tst, suite, opts, debugfs_path);
> +		__igt_kunit(&tst, subtest, opts, debugfs_path, &tests);
>  	}
>  
>  	igt_fixture {
>  		char *suite_name = NULL, *case_name = NULL;
>  
> -		igt_ktap_free(&ktap);
> -
>  		kunit_results_free(&tests, &suite_name, &case_name);
>  
>  		igt_ktest_end(&tst);
>