Re: [igt PATCH 3/4] lib: add subtest extra command line option handling

[Imre Deak <imre.deak@intel.com>]

[-- Attachment #1.1: Type: text/plain, Size: 6054 bytes --]

On Tue, 2013-08-06 at 11:09 +0200, Daniel Vetter wrote:
> On Mon, Aug 05, 2013 at 02:45:25PM +0300, Imre Deak wrote:
> > At the moment any command line option handling done by tests will
> > interfere with the option handling of the subtest interface. To fix this
> > add a new version of the subtest_init function accepting optional short
> > and long command line options. Merge these together with the subtest
> > interface's own long options and handle both together in the same
> > getopt_long call.
> > 
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> 
> Hm, I've thought that getopt would filter the passed-in argv/argc arrays
> and we could run a second getopt afterwards without too much interfence
> (maybe we need to reset a few global getop state variables). But I'm not
> sure since I've never tried it out. Am I wrong?

Afaics getopt itself can't handle the long options (which we already
have for subtests), it'll try to parse each character of the long option
as a short one.

We could still do the scanning twice by always using getopt_long, but
there I don't like the fact that we would have to set opterr=0 and
silently ignore invalid options. Also I thought that later we could add
a check for clashing test case/subtest options and that's not possible
by scanning twice.

--Imre

> -Daniel
> 
> > ---
> >  lib/drmtest.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++++-------
> >  lib/drmtest.h |  6 +++++
> >  2 files changed, 81 insertions(+), 10 deletions(-)
> > 
> > diff --git a/lib/drmtest.c b/lib/drmtest.c
> > index afbaa35..7a68091 100644
> > --- a/lib/drmtest.c
> > +++ b/lib/drmtest.c
> > @@ -657,7 +657,21 @@ void drmtest_stop_signal_helper(void)
> >  static bool list_subtests = false;
> >  static char *run_single_subtest = NULL;
> >  
> > -void drmtest_subtest_init(int argc, char **argv)
> > +static void print_usage(const char *command_str, const char *help_str,
> > +			bool output_on_stderr)
> > +{
> > +	FILE *f = output_on_stderr ? stderr : stdout;
> > +
> > +	fprintf(f, "Usage: %s [OPTIONS]\n"
> > +		   "  --list-subtests\n"
> > +		   "  --run-subtest <pattern>\n"
> > +		   "%s\n", command_str, help_str);
> > +}
> > +
> > +int drmtest_subtest_init_parse_opts(int argc, char **argv, const char *opts,
> > +				    struct option *long_opts,
> > +				    const char *help_str,
> > +				    drmtest_opt_handler_t opt_handler)
> >  {
> >  	int c, option_index = 0;
> >  	static struct option long_options[] = {
> > @@ -665,24 +679,75 @@ void drmtest_subtest_init(int argc, char **argv)
> >  		{"run-subtest", 1, 0, 'r'},
> >  		{NULL, 0, 0, 0,}
> >  	};
> > +	struct option help_opt =
> > +		{"help", 0, 0, 'h'};
> > +	const char *command_str;
> > +	char *short_opts;
> > +	struct option *combined_opts;
> > +	int extra_opts;
> > +	int all_opts;
> > +	int ret = 0;
> > +
> > +	command_str = argv[0];
> > +	if (strrchr(command_str, '/'))
> > +		command_str = strrchr(command_str, '/') + 1;
> > +
> > +	all_opts = 0;
> > +	while (long_opts && long_opts[all_opts].name)
> > +		all_opts++;
> > +	extra_opts = all_opts;
> > +	if (help_str)
> > +		all_opts++;
> > +	all_opts += ARRAY_SIZE(long_options);
> > +
> > +	combined_opts = malloc(all_opts * sizeof(*combined_opts));
> > +	memcpy(combined_opts, long_opts, extra_opts * sizeof(*combined_opts));
> > +	if (help_str) {
> > +		combined_opts[extra_opts] = help_opt;
> > +		extra_opts++;
> > +	}
> > +	memcpy(&combined_opts[extra_opts], long_options,
> > +		ARRAY_SIZE(long_options) * sizeof(*combined_opts));
> >  
> > -	/* supress getopt errors about unknown options */
> > -	opterr = 0;
> > -	/* restrict the option parsing to long option names to avoid collisions
> > -	 * with options the test declares */
> > -	while((c = getopt_long(argc, argv, "",
> > -			       long_options, &option_index)) != -1) {
> > +	ret = asprintf(&short_opts, "%s%s",
> > +		       opts ? opts : "", help_str ? "h" : "");
> > +	assert(ret >= 0);
> > +
> > +	while ((c = getopt_long(argc, argv, short_opts, combined_opts,
> > +			       &option_index)) != -1) {
> >  		switch(c) {
> >  		case 'l':
> > -			list_subtests = true;
> > -			goto out;
> > +			if (!run_single_subtest)
> > +				list_subtests = true;
> > +			break;
> >  		case 'r':
> > -			run_single_subtest = strdup(optarg);
> > +			if (!list_subtests)
> > +				run_single_subtest = strdup(optarg);
> > +			break;
> > +		case '?':
> > +		case 'h':
> > +			print_usage(command_str, help_str, c == '?');
> > +			ret = c == '?' ? -2 : -1;
> >  			goto out;
> > +		default:
> > +			ret = opt_handler(c, option_index);
> > +			if (ret)
> > +				goto out;
> >  		}
> >  	}
> >  
> >  out:
> > +	return ret;
> > +}
> > +
> > +void drmtest_subtest_init(int argc, char **argv)
> > +{
> > +	/* supress getopt errors about unknown options */
> > +	opterr = 0;
> > +
> > +	(void)drmtest_subtest_init_parse_opts(argc, argv, NULL, NULL, NULL,
> > +					      NULL);
> > +
> >  	/* reset opt parsing */
> >  	optind = 1;
> >  }
> > diff --git a/lib/drmtest.h b/lib/drmtest.h
> > index 172bed5..5ca7e2e 100644
> > --- a/lib/drmtest.h
> > +++ b/lib/drmtest.h
> > @@ -94,6 +94,12 @@ void drmtest_progress(const char *header, uint64_t i, uint64_t total);
> >  
> >  /* subtest infrastructure */
> >  void drmtest_subtest_init(int argc, char **argv);
> > +typedef int (*drmtest_opt_handler_t)(int opt, int opt_index);
> > +struct option;
> > +int drmtest_subtest_init_parse_opts(int argc, char **argv, const char *opts,
> > +				    struct option *long_opts,
> > +				    const char *help_str,
> > +				    drmtest_opt_handler_t opt_handler);
> >  bool drmtest_run_subtest(const char *subtest_name);
> >  bool drmtest_only_list_subtests(void);
> >  
> > -- 
> > 1.8.3.2
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx