Re: [LTP] [RFC PATCH 2/5] lib: Allow test to have positional args

[Petr Vorel <pvorel@suse.cz>]

Hi Cyril,

> Hi!
> > diff --git a/doc/developers/writing_tests.rst b/doc/developers/writing_tests.rst
> > index 9b18ec059c..f5796ddc49 100644
> > --- a/doc/developers/writing_tests.rst
> > +++ b/doc/developers/writing_tests.rst
> > @@ -521,7 +521,7 @@ LTP C And Shell Test API Comparison
> >      * - not applicable
> >        - TST_NEEDS_MODULE

> > -    * - not applicable
> > +    * - .pos_args (internal use for tst_run_shell.c)
> >        - TST_POS_ARGS

> >      * - not applicable
> > diff --git a/include/tst_test.h b/include/tst_test.h
> > index eb73cd593c..b249f833ab 100644
> > --- a/include/tst_test.h
> > +++ b/include/tst_test.h
> > @@ -292,8 +292,11 @@ struct tst_fs {
> >   *
> >   * @tcnt: A number of tests. If set the test() callback is called tcnt times
> >   *        and each time passed an increasing counter value.
> > + *
> >   * @options: An NULL optstr terminated array of struct tst_option.
> >   *
> > + * @pos_args: An number of positional parameters passed to tst_run_shell.c.

> We do not support positional arguments for the C API. Do we really need
> them for shell?

I needed this for following change 4th commit ("shell lib: Add basic support for
test cleanup", which wrongly mentions cleanup instead of setup):

-	tst_run_shell $(basename "$0") "$@"
+	tst_run_shell tst_exec.sh $(basename "$0") "$@"

I.e. it is only for tst_run_shell.c.  I'll have look on it, if it's not needed
sure this commit would be useless. I would like to avoid this change as well.

> >   * @min_kver: A minimal kernel version the test can run on. e.g. "3.10".
> >   *
> >   * @supported_archs: A NULL terminated array of architectures the test runs on
> > @@ -528,6 +531,7 @@ struct tst_fs {
> >  	unsigned int tcnt;

> >  	struct tst_option *options;
> > +	int pos_args;

> >  	const char *min_kver;

> > @@ -555,7 +559,6 @@ struct tst_fs {
> >  	unsigned int skip_in_secureboot:1;
> >  	unsigned int skip_in_compat:1;

> > -
> >  	int needs_abi_bits;

> >  	unsigned int needs_hugetlbfs:1;
> > diff --git a/lib/tst_test.c b/lib/tst_test.c
> > index 3823ea109e..1c2cc5e3b2 100644
> > --- a/lib/tst_test.c
> > +++ b/lib/tst_test.c
> > @@ -711,6 +711,9 @@ static void parse_opts(int argc, char *argv[])

> >  	check_option_collision();

> > +	if (tst_test->pos_args < 0)
> > +		tst_brk(TBROK, ".pos_args must be >= 0");

> You can declare pos_args as unsigned and you don't have to add this
> condition.

Good point, thanks!

> >  	optstr[0] = 0;

> >  	for (i = 0; i < ARRAY_SIZE(options); i++)
> > @@ -751,8 +754,10 @@ static void parse_opts(int argc, char *argv[])
> >  		}
> >  	}

> > -	if (optind < argc)
> > -		tst_brk(TBROK, "Unexpected argument(s) '%s'...", argv[optind]);
> > +	if (optind + tst_test->pos_args < argc) {
> > +		tst_brk(TBROK, "Unexpected argument(s) '%s' (%d + %d < %d)",
> > +			argv[optind], optind, tst_test->pos_args, argc);
> > +	}

> And this half enables the positional arguments for the C API as well. If
> we set the pos_args in tst_test, then we can pass them, but there is no
> way how they can be passed to the test.

Again, this is only for tst_run_shell.c.

Kind regards,
Petr

> So if we are going to add them, we should pass then in
> extern char **tst_args or something like that.

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