Re: [PATCH i-g-t] lib: avoid getopt value conflicts with tests

Thomas Wood
2014-07-28T09:38:23Z

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.
>