Re: [LTP] [PATCH] lib: Add checking of setup params in kernel command line

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
> There are some test cases that need to check the configuration
> of setup params, so we add tst_kernel_cmdline_check() to check it
> in do_setup().

Can you elaborate on that? Which tests and what for?

> Signed-off-by: Zhao Gongyi <zhaogongyi@huawei.com>
> ---
>  include/tst_kernel.h |  9 +++++++++
>  include/tst_test.h   |  3 +++
>  lib/tst_kernel.c     | 21 +++++++++++++++++++++
>  lib/tst_test.c       |  9 +++++++++
>  4 files changed, 42 insertions(+)
> 
> diff --git a/include/tst_kernel.h b/include/tst_kernel.h
> index 9e17bb71e..874ec3bf4 100644
> --- a/include/tst_kernel.h
> +++ b/include/tst_kernel.h
> @@ -20,4 +20,13 @@ int tst_kernel_bits(void);
>   */
>  int tst_check_driver(const char *driver);
> 
> +/*
> + * Checks if there is a configuration in the kernel command line.
> + *
> + * @param cmd The setup param that need to be checked in command line.
> + * @return Returns 0 if the kernel configured the setup param in command line.
> + * -1 when the setup param is not configured.
> + */
> +int tst_kernel_cmdline_check(const char *cmd);
> +
>  #endif	/* TST_KERNEL_H__ */
> diff --git a/include/tst_test.h b/include/tst_test.h
> index ac52f268c..ba7d538b7 100644
> --- a/include/tst_test.h
> +++ b/include/tst_test.h
> @@ -312,6 +312,9 @@ struct tst_test {
> 
>  	/* {} terminated array of required CGroup controllers */
>  	const char *const *needs_cgroup_ctrls;
> +
> +	/* NULL terminated array of required kernel cmdlines. */
> +	const char *const *needs_kernel_cmdlines;
>  };
> 
>  /*
> diff --git a/lib/tst_kernel.c b/lib/tst_kernel.c
> index ecf4b917e..1e6d18e55 100644
> --- a/lib/tst_kernel.c
> +++ b/lib/tst_kernel.c
> @@ -24,6 +24,8 @@
>  #include "tst_kernel.h"
>  #include "old_safe_stdio.h"
> 
> +#define COMMAND_LINE_SIZE 4096
> +
>  static int get_kernel_bits_from_uname(struct utsname *buf)
>  {
>  	if (uname(buf)) {
> @@ -189,3 +191,22 @@ int tst_check_driver(const char *driver)
> 
>  	return ret;
>  }
> +
> +int tst_kernel_cmdline_check(const char *cmd)
> +{
> +	char cmdline[COMMAND_LINE_SIZE];
> +
> +	FILE *f = fopen("/proc/cmdline", "r");
> +	if (f == NULL) {
> +		tst_brkm(TBROK | TERRNO, NULL, "Open /proc/cmdline failed");
> +		return -1;
> +	}
> +
> +	while (fscanf(f, "%s", cmdline) > 0) {
> +		if (!strcmp(cmdline, cmd))
> +			return 0;
> +	}
> +
> +	tst_brkm(TCONF, NULL, "Setup param '%s' is not configured", cmd);
> +	return -1;
> +}

If nothing else this call should really take the NULL terminated array
as parameter and check for all flags so that we don't have to parse the
file eac time we check for a flag.

Also there is no point in doing return from a function that calls
tst_brkm(). The function should just return void in this case.

> diff --git a/lib/tst_test.c b/lib/tst_test.c
> index 8ccde1629..7a842c5ae 100644
> --- a/lib/tst_test.c
> +++ b/lib/tst_test.c
> @@ -1125,6 +1125,15 @@ static void do_setup(int argc, char *argv[])
>  	if (tst_test->min_kver)
>  		check_kver();
> 
> +	if (tst_test->needs_kernel_cmdlines)
> +	{
> +		const char *cmd;
> +		int i;
> +
> +		for (i = 0; (cmd = tst_test->needs_kernel_cmdlines[i]); ++i)
> +			tst_kernel_cmdline_check(cmd);
> +	}
> +
>  	if (tst_test->supported_archs && !tst_is_on_arch(tst_test->supported_archs))
>  		tst_brk(TCONF, "This arch '%s' is not supported for test!", tst_arch.name);
> 
> --
> 2.17.1
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

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