Re: [LTP] [PATCH v2 2/4] lib: Introduce LTP_KCONFIG_DISABLE environment variables

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
> diff --git a/doc/user-guide.txt b/doc/user-guide.txt
> index 494652618..8d4435a28 100644
> --- a/doc/user-guide.txt
> +++ b/doc/user-guide.txt
> @@ -18,6 +18,9 @@ For running LTP network tests see `testcases/network/README.md`.
>  | 'LTP_SINGLE_FS_TYPE'  | Testing only - specifies filesystem instead all
>                            supported (for tests with '.all_filesystems').
>  | 'LTP_DEV_FS_TYPE'     | Filesystem used for testing (default: 'ext2').
> +| 'LTP_KCONFIG_DISABLE' | Switch for kernel config check functionality.
> +                          'y' or '1': disable kconfig check,
> +                          'n' or '0': enable kconfig check.

Maybe it would be better named LTP_KCONFIG_SKIP or even
KCONFIG_SKIP_CHECK we do have KCONFIG_PATH so it would make sense to
keep the kernel config related variables prefixed with just KCONFIG_

I think that the point made by Tim Bird was that the KCONFIG_PATH should
be standartized variable among testsuites, so it makes sense to have
KCONFIG_SKIP_CHECK as a standartized variable as well.

[CC: Tim should we start tracking common env variables like this somewhere?]

>  | 'LTP_TIMEOUT_MUL'     | Multiply timeout, must be number >= 1 (> 1 is useful for
>                            slow machines to avoid unexpected timeout).
>                            Variable is also used in shell tests, but ceiled to int.
> diff --git a/lib/tst_kconfig.c b/lib/tst_kconfig.c
> index dc7decff9..c37afd8c8 100644
> --- a/lib/tst_kconfig.c
> +++ b/lib/tst_kconfig.c
> @@ -478,6 +478,27 @@ static void dump_vars(const struct tst_expr *expr)
>  	}
>  }
>  
> +static int tst_kconfig_disabled(void)
> +{
> +	static int check;
> +
> +	if (check)
> +		return check - 1;
> +
> +	char *env = getenv("LTP_KCONFIG_DISABLE");
> +
> +	if (env) {
> +		if (!strcmp(env, "n") || !strcmp(env, "0"))
> +			check = 1;
> +		if (!strcmp(env, "y") || !strcmp(env, "1"))
> +			check = 2;
> +		return check - 1;
> +	}
> +
> +	check = 1;
> +	return 0;
> +}

As pointed out by Peter, can we please keep it simple? I.e. just branch
on variable defined/undefined?

>  int tst_kconfig_check(const char *const kconfigs[])
>  {
>  	size_t expr_cnt = array_len(kconfigs);
> @@ -485,6 +506,11 @@ int tst_kconfig_check(const char *const kconfigs[])
>  	unsigned int i, var_cnt;
>  	int ret = 0;
>  
> +	if (tst_kconfig_disabled()) {
> +		tst_res(TINFO, "Kernel config check functionality has been disabled.");

I would try to make more clear what has happened here, something as:

"Skipping kernel config check as requested"

Or something along these lines.

> +		return 0;
> +	}
> +
>  	for (i = 0; i < expr_cnt; i++) {
>  		exprs[i] = tst_bool_expr_parse(kconfigs[i]);

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp