On Tue, Dec 09, 2014 at 09:01:54PM -0500, Rodrigo Vivi wrote:
> Use cmdline variable for interactive debug instead of env var.
>
> Signed-off-by: Rodrigo Vivi <rodrigo.v...@intel.com>
Looks nice, two small comments below.
> ---
> lib/igt_aux.c | 20 ++++++++++----------
> lib/igt_aux.h | 2 +-
> lib/igt_core.c | 6 ++++++
> lib/igt_core.h | 2 ++
> 4 files changed, 19 insertions(+), 11 deletions(-)
>
> diff --git a/lib/igt_aux.c b/lib/igt_aux.c
> index 49d1ec4..ff668d4 100644
> --- a/lib/igt_aux.c
> +++ b/lib/igt_aux.c
> @@ -372,32 +372,32 @@ void igt_drop_root(void)
>
> /**
> * igt_debug_wait_for_keypress:
> - * @key: env var lookup to to enable this wait
> + * @var: var lookup to to enable this wait
> *
> * Waits for a key press when run interactively and when the corresponding
> debug
> - * key is set in the IGT_DEBUG_INTERACTIVE environment variable. Multiple
> keys
> + * var is set in the --interactive-debug=<var> variable. Multiple keys
> * can be specified as a comma-separated list or alternatively "all" if a
> wait
> - * should happen for all keys. When not connected to a terminal the
> environment
> - * setting is ignored and execution immediately continues.
> + * should happen for all cases.
> + *
> + * When not connected to a terminal interactive_debug is ignored
> + * and execution immediately continues.
> *
> * This is useful for display tests where under certain situation manual
> * inspection of the display is useful. Or when running a testcase in the
> * background.
> */
> -void igt_debug_wait_for_keypress(const char *key)
> +void igt_debug_wait_for_keypress(const char *var)
> {
> struct termios oldt, newt;
> - const char *env;
>
> if (!isatty(STDIN_FILENO))
> return;
>
> - env = getenv("IGT_DEBUG_INTERACTIVE");
> -
> - if (!env)
> + if (!igt_interactive_debug)
> return;
>
> - if (!strstr(env, key) && !strstr(env, "all"))
> + if (!strstr(igt_interactive_debug, var) &&
> + !strstr(igt_interactive_debug, "all"))
> return;
>
> igt_info("Press any key to continue ...\n");
> diff --git a/lib/igt_aux.h b/lib/igt_aux.h
> index 63e1b06..59022cd 100644
> --- a/lib/igt_aux.h
> +++ b/lib/igt_aux.h
> @@ -60,7 +60,7 @@ void igt_system_suspend_autoresume(void);
> /* dropping priviledges */
> void igt_drop_root(void);
>
> -void igt_debug_wait_for_keypress(const char *key);
> +void igt_debug_wait_for_keypress(const char *var);
>
> enum igt_runtime_pm_status {
> IGT_RUNTIME_PM_STATUS_ACTIVE,
> diff --git a/lib/igt_core.c b/lib/igt_core.c
> index 13a52a5..461b1d3 100644
> --- a/lib/igt_core.c
> +++ b/lib/igt_core.c
> @@ -225,6 +225,7 @@ enum {
> OPT_RUN_SUBTEST,
> OPT_DESCRIPTION,
> OPT_DEBUG,
> + OPT_INTERACTIVE_DEBUG,
> OPT_HELP = 'h'
> };
>
> @@ -391,6 +392,7 @@ static void print_usage(const char *help_str, bool
> output_on_stderr)
> fprintf(f, " --list-subtests\n"
> " --run-subtest <pattern>\n"
> " --debug\n"
> + " --interactive-debug <pattern>\n"
> " --help-description\n"
> " --help\n");
> if (help_str)
> @@ -423,6 +425,7 @@ static int common_init(int argc, char **argv,
> {"run-subtest", 1, 0, OPT_RUN_SUBTEST},
> {"help-description", 0, 0, OPT_DESCRIPTION},
> {"debug", 0, 0, OPT_DEBUG},
> + {"interactive-debug", 1, 0, OPT_INTERACTIVE_DEBUG},
> {"help", 0, 0, OPT_HELP},
> {0, 0, 0, 0}
> };
> @@ -508,6 +511,9 @@ static int common_init(int argc, char **argv,
> while ((c = getopt_long(argc, argv, short_opts, combined_opts,
> &option_index)) != -1) {
> switch(c) {
> + case OPT_INTERACTIVE_DEBUG:
> + igt_interactive_debug = strdup(optarg);;
> + break;
Hm, while we change this to cmdline option I think we should make the
optarg optional and if it's not set, set it to "all".
> case OPT_DEBUG:
> igt_log_level = IGT_LOG_DEBUG;
> break;
> diff --git a/lib/igt_core.h b/lib/igt_core.h
> index a258348..20942e4 100644
> --- a/lib/igt_core.h
> +++ b/lib/igt_core.h
> @@ -511,6 +511,8 @@ bool igt_run_in_simulation(void);
>
> void igt_skip_on_simulation(void);
>
> +const char *igt_interactive_debug;
Can you please add a bit of gtkdoc for this, too, now that it's exported.
Thanks, Daniel
> +
> /* structured logging */
> enum igt_log_level {
> IGT_LOG_DEBUG,
> --
> 1.9.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
