| Message ID | 1406304519-7100-1-git-send-email-thomas.wood@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
2014-07-25 13:08 GMT-03:00 Thomas Wood <thomas.wood@intel.com>: > Most tests use a printable character as the value for getopt to return, > so avoid conflicts by using non-printing values for the standard options. Instead of this patch, isn't there any way to verify if the tests are using any character that is "reserved" to these general options? Having "-r" instead of "--run-subtest" is quite nice. That said, I'm not against your patch either. > > Signed-off-by: Thomas Wood <thomas.wood@intel.com> > --- > lib/igt_core.c | 23 +++++++++++++++-------- > 1 file changed, 15 insertions(+), 8 deletions(-) > > diff --git a/lib/igt_core.c b/lib/igt_core.c > index a0c9499..882614a 100644 > --- a/lib/igt_core.c > +++ b/lib/igt_core.c > @@ -218,6 +218,13 @@ int num_test_children; > int test_children_sz; > bool test_child; > > +enum { > + OPT_LIST_SUBTESTS, > + OPT_RUN_SUBTEST, > + OPT_DEBUG, > + OPT_HELP > +}; > + > __attribute__((format(printf, 1, 2))) > static void kmsg(const char *format, ...) > #define KERN_INFO "<5>" > @@ -320,10 +327,10 @@ static int common_init(int argc, char **argv, > { > int c, option_index = 0; > static struct option long_options[] = { > - {"list-subtests", 0, 0, 'l'}, > - {"run-subtest", 1, 0, 'r'}, > - {"debug", 0, 0, 'd'}, > - {"help", 0, 0, 'h'}, > + {"list-subtests", 0, 0, OPT_LIST_SUBTESTS}, > + {"run-subtest", 1, 0, OPT_RUN_SUBTEST}, > + {"debug", 0, 0, OPT_DEBUG}, > + {"help", 0, 0, OPT_HELP}, > }; > char *short_opts; > struct option *combined_opts; > @@ -370,18 +377,18 @@ static int common_init(int argc, char **argv, > while ((c = getopt_long(argc, argv, short_opts, combined_opts, > &option_index)) != -1) { > switch(c) { > - case 'd': > + case OPT_DEBUG: > igt_log_level = IGT_LOG_DEBUG; > break; > - case 'l': > + case OPT_LIST_SUBTESTS: > if (!run_single_subtest) > list_subtests = true; > break; > - case 'r': > + case OPT_RUN_SUBTEST: > if (!list_subtests) > run_single_subtest = strdup(optarg); > break; > - case 'h': > + case OPT_HELP: > print_usage(help_str, false); > ret = -1; > goto out; > -- > 1.9.3 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On 25 July 2014 17:57, Paulo Zanoni <przanoni@gmail.com> wrote: > 2014-07-25 13:08 GMT-03:00 Thomas Wood <thomas.wood@intel.com>: >> Most tests use a printable character as the value for getopt to return, >> so avoid conflicts by using non-printing values for the standard options. > > Instead of this patch, isn't there any way to verify if the tests are > using any character that is "reserved" to these general options? > Having "-r" instead of "--run-subtest" is quite nice. That said, I'm > not against your patch either. The only standard short option is "-h" for "--help", which needs to be fixed in the patch. Adding a warning for conflicting short options and getopt return values could be added, but probably in a separate patch. > >> >> Signed-off-by: Thomas Wood <thomas.wood@intel.com> >> --- >> lib/igt_core.c | 23 +++++++++++++++-------- >> 1 file changed, 15 insertions(+), 8 deletions(-) >> >> diff --git a/lib/igt_core.c b/lib/igt_core.c >> index a0c9499..882614a 100644 >> --- a/lib/igt_core.c >> +++ b/lib/igt_core.c >> @@ -218,6 +218,13 @@ int num_test_children; >> int test_children_sz; >> bool test_child; >> >> +enum { >> + OPT_LIST_SUBTESTS, >> + OPT_RUN_SUBTEST, >> + OPT_DEBUG, >> + OPT_HELP >> +}; >> + >> __attribute__((format(printf, 1, 2))) >> static void kmsg(const char *format, ...) >> #define KERN_INFO "<5>" >> @@ -320,10 +327,10 @@ static int common_init(int argc, char **argv, >> { >> int c, option_index = 0; >> static struct option long_options[] = { >> - {"list-subtests", 0, 0, 'l'}, >> - {"run-subtest", 1, 0, 'r'}, >> - {"debug", 0, 0, 'd'}, >> - {"help", 0, 0, 'h'}, >> + {"list-subtests", 0, 0, OPT_LIST_SUBTESTS}, >> + {"run-subtest", 1, 0, OPT_RUN_SUBTEST}, >> + {"debug", 0, 0, OPT_DEBUG}, >> + {"help", 0, 0, OPT_HELP}, >> }; >> char *short_opts; >> struct option *combined_opts; >> @@ -370,18 +377,18 @@ static int common_init(int argc, char **argv, >> while ((c = getopt_long(argc, argv, short_opts, combined_opts, >> &option_index)) != -1) { >> switch(c) { >> - case 'd': >> + case OPT_DEBUG: >> igt_log_level = IGT_LOG_DEBUG; >> break; >> - case 'l': >> + case OPT_LIST_SUBTESTS: >> if (!run_single_subtest) >> list_subtests = true; >> break; >> - case 'r': >> + case OPT_RUN_SUBTEST: >> if (!list_subtests) >> run_single_subtest = strdup(optarg); >> break; >> - case 'h': >> + case OPT_HELP: >> print_usage(help_str, false); >> ret = -1; >> goto out; >> -- >> 1.9.3 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > Paulo Zanoni > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > --------------------------------------------------------------------- > Intel Corporation (UK) Limited > Registered No. 1134945 (England) > Registered Office: Pipers Way, Swindon SN3 1RJ > VAT No: 860 2173 47 > > This e-mail and any attachments may contain confidential material for > the sole use of the intended recipient(s). Any review or distribution > by others is strictly prohibited. If you are not the intended > recipient, please contact the sender and delete all copies. >
diff --git a/lib/igt_core.c b/lib/igt_core.c index a0c9499..882614a 100644 --- a/lib/igt_core.c +++ b/lib/igt_core.c @@ -218,6 +218,13 @@ int num_test_children; int test_children_sz; bool test_child; +enum { + OPT_LIST_SUBTESTS, + OPT_RUN_SUBTEST, + OPT_DEBUG, + OPT_HELP +}; + __attribute__((format(printf, 1, 2))) static void kmsg(const char *format, ...) #define KERN_INFO "<5>" @@ -320,10 +327,10 @@ static int common_init(int argc, char **argv, { int c, option_index = 0; static struct option long_options[] = { - {"list-subtests", 0, 0, 'l'}, - {"run-subtest", 1, 0, 'r'}, - {"debug", 0, 0, 'd'}, - {"help", 0, 0, 'h'}, + {"list-subtests", 0, 0, OPT_LIST_SUBTESTS}, + {"run-subtest", 1, 0, OPT_RUN_SUBTEST}, + {"debug", 0, 0, OPT_DEBUG}, + {"help", 0, 0, OPT_HELP}, }; char *short_opts; struct option *combined_opts; @@ -370,18 +377,18 @@ static int common_init(int argc, char **argv, while ((c = getopt_long(argc, argv, short_opts, combined_opts, &option_index)) != -1) { switch(c) { - case 'd': + case OPT_DEBUG: igt_log_level = IGT_LOG_DEBUG; break; - case 'l': + case OPT_LIST_SUBTESTS: if (!run_single_subtest) list_subtests = true; break; - case 'r': + case OPT_RUN_SUBTEST: if (!list_subtests) run_single_subtest = strdup(optarg); break; - case 'h': + case OPT_HELP: print_usage(help_str, false); ret = -1; goto out;
Most tests use a printable character as the value for getopt to return, so avoid conflicts by using non-printing values for the standard options. Signed-off-by: Thomas Wood <thomas.wood@intel.com> --- lib/igt_core.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-)