[PATCH i-g-t 2/2] lib/igt_kmod: Simplify kunit_set_filtering()

[Janusz Krzysztofik]

Hi Lucas,

On Thursday, 31 October 2024 01:31:42 CET Lucas De Marchi wrote:
> igt_sysfs_set() already supports setting a 0-length value since commit
> 1c0b492a0b5c ("lib/igt_sysfs: make sure to write empty strings").
> Remove workaround in kunit_set_filtering() and simplify.
> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>

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

> ---
>  lib/igt_kmod.c | 40 +++++-----------------------------------
>  1 file changed, 5 insertions(+), 35 deletions(-)
> 
> diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
> index 4cb9e886d..5fd500384 100644
> --- a/lib/igt_kmod.c
> +++ b/lib/igt_kmod.c
> @@ -939,7 +939,7 @@ static bool kunit_set_filtering(const char *filter_glob, const char *filter,
>  				const char *filter_action)
>  {
>  	int params;
> -	bool ret;
> +	bool ret = true;

NIT: Preserve Christmas tree order.

Thanks,
Janusz

>  
>  	params = open_parameters("kunit");
>  	if (igt_debug_on(params < 0))
> @@ -952,41 +952,11 @@ static bool kunit_set_filtering(const char *filter_glob, const char *filter,
>  	 * As a workaround, we use non-NULL strings that exhibit
>  	 * the same behaviour as if default NULLs were in place.
>  	 */
> -	ret = !igt_debug_on(!igt_sysfs_set(params, "filter_glob",
> -					   filter_glob ?: "*"));
> -	if (!ret)
> -		goto close;
> +	if (igt_debug_on(!igt_sysfs_set(params, "filter_glob", filter_glob ?: "*")) ||
> +	    igt_debug_on(!igt_sysfs_set(params, "filter", filter ?: "module!=none")) ||
> +	    igt_debug_on(!igt_sysfs_set(params, "filter_action", filter_action ?: "")))
> +		ret = false;
>  
> -	ret = !igt_debug_on(!igt_sysfs_set(params, "filter",
> -					   filter ?: "module!=none"));
> -	if (!ret)
> -		goto close;
> -
> -	ret = !igt_debug_on(!igt_sysfs_set(params, "filter_action",
> -					   filter_action ?: ""));
> -
> -	/*
> -	 * TODO: Drop the extra check below as soon as igt_sysfs_set()
> -	 *	 can correctly process empty strings which we are using
> -	 *	 as filter_action NULL equivalent.
> -	 *
> -	 * We need this check only when NULL is requested for "filter_action"
> -	 * and not for "filter" parameter, otherwise, even if "filter_action"
> -	 * was previously set to "skip", we don't care since our
> -	 * "module!=none" default filter guarantees that no test cases are
> -	 * filtered out to be processed as "filter_action" says.
> -	 */
> -	if (ret && !filter_action && filter) {
> -		filter_action = igt_sysfs_get(params, "filter_action");
> -
> -		ret = !(igt_debug_on_f(!filter_action,
> -				       "open() failed\n") ||
> -			igt_debug_on_f(strlen(filter_action),
> -				       "empty string not applied\n"));
> -		free((char *)filter_action);
> -	}
> -
> -close:
>  	close(params);
>  
>  	return ret;
>