Re: [LTP] [PATCH v2 1/2] library: add cmd check handler in needs_cmds

[Petr Vorel <pvorel@suse.cz>]

Hi Xu, Cyril,

> Testcase ie statx05 needs mkfs.ext4 >= 1.43.0 because of encrypt feature.

> As Cyril suggested, add cmd check handler in needs_cmd.

Great idea, I have something like this in my TODO list as well, glad I can
delete it :).

> We don't use tst_ prefix ie tst_check_cmd since we don't export this api to user.
> This check_cmd not only check cmd whether existed but also check the cmd version whether
> meet test's requirement.

> In check_cmd function, use strtok_r to split cmd_token,op_token,version_token.
> It only supports six operations '>=' '<=' '>' '<' '==' '!='.

> Currently, for the command version check, it only supports  mkfs.ext4 command. If you
> want to support more commands, just add your own .parser and .table_get methond in
> version_parsers structure.

> Suggested-by: Cyril Hrubis <chrubis@suse.cz>
> Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
> ---
> v1->v2
> 1. rename tst_version_parser to check_cmd
Why not tst_cmd_check(), i.e. using tst_ prefix?

+1 for moving it into tst_cmd.c.


> 2. For mkfs_ext4_version_table_get method, use sscanf instead of strtok_r
> 3. use enum for cmd op
> 4. fix description
> 5. add more newlib test for this
>  doc/c-test-api.txt                   |  14 +++
>  lib/newlib_tests/.gitignore          |   8 ++
>  lib/newlib_tests/test_needs_cmds01.c |  25 ++++
>  lib/newlib_tests/test_needs_cmds02.c |  24 ++++
>  lib/newlib_tests/test_needs_cmds03.c |  24 ++++
>  lib/newlib_tests/test_needs_cmds04.c |  24 ++++
>  lib/newlib_tests/test_needs_cmds05.c |  24 ++++
>  lib/newlib_tests/test_needs_cmds06.c |  24 ++++
>  lib/newlib_tests/test_needs_cmds07.c |  24 ++++
>  lib/newlib_tests/test_needs_cmds08.c |  27 +++++
Also, could you please put tests which expect TPASS or TCONF into
lib/newlib_tests/runtest.sh?


> diff --git a/lib/tst_test.c b/lib/tst_test.c
> index a79275722..7cca209ab 100644
> --- a/lib/tst_test.c
> +++ b/lib/tst_test.c
> @@ -65,6 +65,15 @@ struct results {
>  	unsigned int timeout;
>  };

> +enum cmd_op {
> +	OP_GE, /* >= */
> +	OP_GT, /* >  */
> +	OP_LE, /* <= */
> +	OP_LT, /* <  */
> +	OP_EQ, /* == */
> +	OP_NE, /* != */
> +};
> +
>  static struct results *results;

>  static int ipc_fd;
> @@ -950,6 +959,162 @@ static void prepare_device(void)
>  	}
>  }

> +static int mkfs_ext4_version_parser(void)
> +{
> +	FILE *f;
> +	int rc, major, minor, patch;
> +
> +	f = popen("mkfs.ext4 -V 2>&1", "r");
> +	if (!f) {
> +		tst_res(TWARN, "Could not run mkfs.ext4 -V 2>&1 cmd");
> +		return -1;
> +	}
> +	rc = fscanf(f, "mke2fs %d.%d.%d", &major, &minor, &patch);

I guess many functions will have X.Y.Z format. Maybe later we could have generic
functions similar to kernel SYSCALL_DEFINEn() macros, passing them just
necessary format string.  At least that was what I had in my mind when thinking
about this.

> +	pclose(f);
> +	if (rc != 3) {
> +		tst_res(TWARN, "Unable to parse mkfs.ext4 version");
> +		return -1;
> +	}
> +
> +	return major * 10000 +  minor * 100 + patch;
> +}
> +
> +static int mkfs_ext4_version_table_get(char *version)
> +{
> +	int major, minor, patch;
> +	int len;
> +
> +	if (sscanf(version, "%u.%u.%u %n", &major, &minor, &patch, &len) != 3) {
> +		tst_res(TWARN, "Illega version(%s), "
typo s/Illega/Illegal/

> +			"should use format like 1.43.0", version);
nit: I'd keep string on single line (easier to grep and it's not too long being
on single line like the others below).

Kind regards,
Petr

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