Re: [PATCH] intel-gpu-tools: remove the --cmd option from gem_seqno_wrap

[Mika Kuoppala <mika.kuoppala@linux.intel.com>]

tim.gore@intel.com writes:

> From: Tim Gore <tim.gore@intel.com>
>
> gem_seqno_wrap was not being built on Android because it uses
> wordexp which is not in Bionic.
> After discussion with Mika Kuoppala (the test author) it seems
> that wordexp was used to implement the --cmd option that was
> really only intended for use during development of the test and
> is no longer needed. So I have removed support for this option
> and enabled this test for Android.
>
> Signed-off-by: Tim Gore <tim.gore@intel.com>

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

> ---
>  tests/Android.mk       |  1 -
>  tests/gem_seqno_wrap.c | 77 ++------------------------------------------------
>  2 files changed, 3 insertions(+), 75 deletions(-)
>
> diff --git a/tests/Android.mk b/tests/Android.mk
> index 9db6625..f085d35 100644
> --- a/tests/Android.mk
> +++ b/tests/Android.mk
> @@ -28,7 +28,6 @@ endef
>  
>  # some tests still do not build under android
>  skip_tests_list :=
> -skip_tests_list += gem_seqno_wrap
>  skip_tests_list += testdisplay        # needs glib.h
>  skip_tests_list += pm_rpm
>  
> diff --git a/tests/gem_seqno_wrap.c b/tests/gem_seqno_wrap.c
> index fa38f1f..ad4f839 100644
> --- a/tests/gem_seqno_wrap.c
> +++ b/tests/gem_seqno_wrap.c
> @@ -38,7 +38,6 @@
>  #include <sys/types.h>
>  #include <sys/wait.h>
>  #include <limits.h>
> -#include <wordexp.h>
>  #include <getopt.h>
>  #include <signal.h>
>  #include <errno.h>
> @@ -62,7 +61,6 @@ static struct intel_batchbuffer *batch_3d;
>  struct option_struct {
>  	int rounds;
>  	int background;
> -	char cmd[1024];
>  	int timeout;
>  	int dontwrap;
>  	int prewrap_space;
> @@ -281,64 +279,6 @@ static void run_sync_test(int num_buffers, bool verify)
>  	close(fd);
>  }
>  
> -static int run_cmd(char *s)
> -{
> -	int pid;
> -	int r = -1;
> -	int status = 0;
> -	wordexp_t wexp;
> -	int i;
> -	r = wordexp(s, &wexp, 0);
> -	if (r != 0) {
> -		printf("can't parse %s\n", s);
> -		return r;
> -	}
> -
> -	for(i = 0; i < wexp.we_wordc; i++)
> -		printf("argv[%d] = %s\n", i, wexp.we_wordv[i]);
> -
> -	pid = fork();
> -
> -	if (pid == 0) {
> -		char path[PATH_MAX];
> -		char full_path[PATH_MAX];
> -
> -		if (getcwd(path, PATH_MAX) == NULL)
> -			perror("getcwd");
> -
> -		igt_assert(snprintf(full_path, PATH_MAX, "%s/%s", path, wexp.we_wordv[0]) > 0);
> -
> -		r = execv(full_path, wexp.we_wordv);
> -		if (r == -1)
> -			perror("execv failed");
> -	} else {
> -		int waitcount = options.timeout;
> -
> -		while(waitcount-- > 0) {
> -			r = waitpid(pid, &status, WNOHANG);
> -			if (r == pid) {
> -				if(WIFEXITED(status)) {
> -					if (WEXITSTATUS(status))
> -						fprintf(stderr,
> -						    "child returned with %d\n",
> -							WEXITSTATUS(status));
> -					return WEXITSTATUS(status);
> -				}
> -			} else if (r != 0) {
> -				perror("waitpid");
> -				return -errno;
> -			}
> -
> -			sleep(3);
> -		}
> -
> -		kill(pid, SIGKILL);
> -		return -ETIMEDOUT;
> -	}
> -
> -	return r;
> -}
> -
>  static const char *dfs_base = "/sys/kernel/debug/dri";
>  static const char *dfs_entry = "i915_next_seqno";
>  
> @@ -415,7 +355,7 @@ static int write_seqno(uint32_t seqno)
>  	int fh;
>  	char buf[32];
>  	int r;
> -	uint32_t rb;
> +	uint32_t rb = -1;
>  
>  	if (options.dontwrap)
>  		return 0;
> @@ -461,10 +401,7 @@ static uint32_t calc_prewrap_val(void)
>  
>  static void run_test(void)
>  {
> -	if (strnlen(options.cmd, sizeof(options.cmd)) > 0)
> -		igt_assert(run_cmd(options.cmd) == 0);
> -	else
> -		run_sync_test(options.buffers, true);
> +	run_sync_test(options.buffers, true);
>  }
>  
>  static void preset_run_once(void)
> @@ -521,7 +458,6 @@ static void print_usage(const char *s)
>  	printf("%s: [OPTION]...\n", s);
>  	printf("    where options are:\n");
>  	printf("    -b --background       run in background inducing wraps\n");
> -	printf("    -c --cmd=cmdstring    use cmdstring to cross wrap\n");
>  	printf("    -n --rounds=num       run num times across wrap boundary, 0 == forever\n");
>  	printf("    -t --timeout=sec      set timeout to wait for testrun to sec seconds\n");
>  	printf("    -d --dontwrap         don't wrap just run the test\n");
> @@ -536,7 +472,6 @@ static void parse_options(int argc, char **argv)
>  	int c;
>  	int option_index = 0;
>  	static struct option long_options[] = {
> -		{"cmd", required_argument, 0, 'c'},
>  		{"rounds", required_argument, 0, 'n'},
>  		{"background", no_argument, 0, 'b'},
>  		{"timeout", required_argument, 0, 't'},
> @@ -546,7 +481,6 @@ static void parse_options(int argc, char **argv)
>  		{"buffers", required_argument, 0, 'i'},
>  	};
>  
> -	strcpy(options.cmd, "");
>  	options.rounds = SLOW_QUICK(50, 2);
>  	options.background = 0;
>  	options.dontwrap = 0;
> @@ -555,7 +489,7 @@ static void parse_options(int argc, char **argv)
>  	options.prewrap_space = 21;
>  	options.buffers = 10;
>  
> -	while((c = getopt_long(argc, argv, "c:n:bvt:dp:ri:",
> +	while((c = getopt_long(argc, argv, "n:bvt:dp:ri:",
>  			       long_options, &option_index)) != -1) {
>  		switch(c) {
>  		case 'b':
> @@ -570,11 +504,6 @@ static void parse_options(int argc, char **argv)
>  			options.rounds = atoi(optarg);
>  			printf("running %d rounds\n", options.rounds);
>  			break;
> -		case 'c':
> -			strncpy(options.cmd, optarg, sizeof(options.cmd) - 1);
> -			options.cmd[sizeof(options.cmd) - 1] = 0;
> -			printf("cmd set to %s\n", options.cmd);
> -			break;
>  		case 'i':
>  			options.buffers = atoi(optarg);
>  			printf("buffers %d\n", options.buffers);
> -- 
> 1.9.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx