[PATCH i-g-t 0/5] Assorted compile warnings fixes

[PATCH i-g-t 0/5] Assorted compile warnings fixes

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

As the subject says.

Even if some are not perfect I doubt they make anything worse.

Tvrtko Ursulin (5):
  intel-gpu-overlay: Avoid theoretical string overflow
  aubdump: Avoid mixing declarations and code
  lib/core: Avoid unused result in backtrace printing
  gem_syslatency: Avoid arithmetic on void * warning
  intel_gvtg_test: Handle system(3) return value.

 benchmarks/gem_syslatency.c |  2 +-
 lib/igt_core.c              | 23 ++++++++++++++++-------
 overlay/overlay.c           |  6 +++---
 tools/aubdump.c             |  5 +++--
 tools/intel_gvtg_test.c     |  4 +++-
 5 files changed, 26 insertions(+), 14 deletions(-)

-- 
2.14.1

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

[PATCH i-g-t 1/5] intel-gpu-overlay: Avoid theoretical string overflow

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

GCC 7 notices a theoretical string overflow. Use snprintf and increase
the buffer to avoid that.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 overlay/overlay.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/overlay/overlay.c b/overlay/overlay.c
index 5fc6958be2d9..2ec86609a464 100644
--- a/overlay/overlay.c
+++ b/overlay/overlay.c
@@ -743,7 +743,7 @@ static void init_gem_objects(struct overlay_context *ctx,
 static void show_gem_objects(struct overlay_context *ctx, struct overlay_gem_objects *go)
 {
 	struct gem_objects_comm *comm;
-	char buf[160];
+	char buf[360];
 	cairo_pattern_t *linear;
 	int x, y, y1, y2;
 
@@ -785,7 +785,7 @@ static void show_gem_objects(struct overlay_context *ctx, struct overlay_gem_obj
 	cairo_pattern_destroy(linear);
 	cairo_fill(ctx->cr);
 
-	sprintf(buf, "Total: %ldMB, %ld objects",
+	snprintf(buf, sizeof(buf), "Total: %luMB, %lu objects",
 		go->gem_objects.total_bytes >> 20, go->gem_objects.total_count);
 	cairo_set_source_rgba(ctx->cr, 1, 1, 1, 1);
 	cairo_move_to(ctx->cr, x, y);
@@ -798,7 +798,7 @@ static void show_gem_objects(struct overlay_context *ctx, struct overlay_gem_obj
 		if ((comm->bytes >> 20) == 0)
 			break;
 
-		sprintf(buf, "%s %ldMB, %ld objects",
+		snprintf(buf, sizeof(buf), "%s %luMB, %lu objects",
 			comm->name, comm->bytes >> 20, comm->count);
 		cairo_move_to(ctx->cr, x, y);
 		cairo_show_text(ctx->cr, buf);
-- 
2.14.1

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

Re: [PATCH i-g-t 1/5] intel-gpu-overlay: Avoid theoretical string overflow

[Petri Latvala]

On Wed, Nov 08, 2017 at 12:06:50PM +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> GCC 7 notices a theoretical string overflow. Use snprintf and increase
> the buffer to avoid that.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Reviewed-by: Petri Latvala <petri.latvala@intel.com>


> ---
>  overlay/overlay.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/overlay/overlay.c b/overlay/overlay.c
> index 5fc6958be2d9..2ec86609a464 100644
> --- a/overlay/overlay.c
> +++ b/overlay/overlay.c
> @@ -743,7 +743,7 @@ static void init_gem_objects(struct overlay_context *ctx,
>  static void show_gem_objects(struct overlay_context *ctx, struct overlay_gem_objects *go)
>  {
>  	struct gem_objects_comm *comm;
> -	char buf[160];
> +	char buf[360];
>  	cairo_pattern_t *linear;
>  	int x, y, y1, y2;
>  
> @@ -785,7 +785,7 @@ static void show_gem_objects(struct overlay_context *ctx, struct overlay_gem_obj
>  	cairo_pattern_destroy(linear);
>  	cairo_fill(ctx->cr);
>  
> -	sprintf(buf, "Total: %ldMB, %ld objects",
> +	snprintf(buf, sizeof(buf), "Total: %luMB, %lu objects",
>  		go->gem_objects.total_bytes >> 20, go->gem_objects.total_count);
>  	cairo_set_source_rgba(ctx->cr, 1, 1, 1, 1);
>  	cairo_move_to(ctx->cr, x, y);
> @@ -798,7 +798,7 @@ static void show_gem_objects(struct overlay_context *ctx, struct overlay_gem_obj
>  		if ((comm->bytes >> 20) == 0)
>  			break;
>  
> -		sprintf(buf, "%s %ldMB, %ld objects",
> +		snprintf(buf, sizeof(buf), "%s %luMB, %lu objects",
>  			comm->name, comm->bytes >> 20, comm->count);
>  		cairo_move_to(ctx->cr, x, y);
>  		cairo_show_text(ctx->cr, buf);
> -- 
> 2.14.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t 2/5] aubdump: Avoid mixing declarations and code

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Avoids the build warning about ISO C90 forbiding mixing the two.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 tools/aubdump.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/aubdump.c b/tools/aubdump.c
index 1aeff5f4532c..27709a17abbc 100644
--- a/tools/aubdump.c
+++ b/tools/aubdump.c
@@ -407,6 +407,7 @@ dump_execbuffer2(int fd, struct drm_i915_gem_execbuffer2 *execbuffer2)
 	uint32_t offset = gtt_size();
 	struct drm_i915_gem_exec_object2 *obj;
 	struct bo *bo, *batch_bo;
+	int batch_index;
 	void *data;
 
 	/* We can't do this at open time as we're not yet authenticated. */
@@ -460,8 +461,8 @@ dump_execbuffer2(int fd, struct drm_i915_gem_execbuffer2 *execbuffer2)
 		fail_if(bo->map == MAP_FAILED, "intel_aubdump: bo mmap failed\n");
 	}
 
-	int batch_index = (execbuffer2->flags & I915_EXEC_BATCH_FIRST) ? 0 :
-			  execbuffer2->buffer_count - 1;
+	batch_index = (execbuffer2->flags & I915_EXEC_BATCH_FIRST) ? 0 :
+		      execbuffer2->buffer_count - 1;
 	batch_bo = get_bo(exec_objects[batch_index].handle);
 	for (uint32_t i = 0; i < execbuffer2->buffer_count; i++) {
 		obj = &exec_objects[i];
-- 
2.14.1

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

[PATCH i-g-t 3/5] lib/core: Avoid unused result in backtrace printing

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Add helpers for direct write to stderr to consolidate the code
and avoid the unused result warning in build.

v2: Use igt_ignore_warn since it is questionable whether igt_assert
    is safe in signal handlers. (Chris Wilson)

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 lib/igt_core.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/lib/igt_core.c b/lib/igt_core.c
index 538a4472e209..e866bf995872 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -1239,7 +1239,7 @@ static const char hex[] = "0123456789abcdef";
 static void
 xputch(int c)
 {
-	write(STDERR_FILENO, (const void *) &c, 1);
+	igt_ignore_warn(write(STDERR_FILENO, (const void *) &c, 1));
 }
 
 static int
@@ -1384,13 +1384,23 @@ xprintf(const char *fmt, ...)
 	va_end(ap);
 }
 
+static void __write_stderr(const char *str, size_t len)
+{
+	igt_ignore_warn(write(STDERR_FILENO, str, len));
+}
+
+static void write_stderr(const char *str)
+{
+	__write_stderr(str, strlen(str));
+}
+
 static void print_backtrace_sig_safe(void)
 {
 	unw_cursor_t cursor;
 	unw_context_t uc;
 	int stack_num = 0;
 
-	write(STDERR_FILENO, "Stack trace: \n", 15);
+	write_stderr("Stack trace: \n");
 
 	unw_getcontext(&uc);
 	unw_init_local(&cursor, &uc);
@@ -1899,11 +1909,10 @@ static void fatal_sig_handler(int sig)
 			continue;
 
 		if (handled_signals[i].name_len) {
-			igt_assert_eq(write(STDERR_FILENO, "Received signal ", 16),
-                                      16);
-			igt_assert_eq(write(STDERR_FILENO, handled_signals[i].name, handled_signals[i].name_len),
-                                      handled_signals[i].name_len);
-			igt_assert_eq(write(STDERR_FILENO, ".\n", 2), 2);
+			write_stderr("Received signal ");
+			__write_stderr(handled_signals[i].name,
+				       handled_signals[i].name_len);
+			write_stderr(".\n");
 		}
 
 		if (crash_signal(sig)) {
-- 
2.14.1

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

Re: [PATCH i-g-t 3/5] lib/core: Avoid unused result in backtrace printing

[Tvrtko Ursulin]

Ping on this one - do we want to squash these compile warnings, yes? :)

On 08/11/2017 12:06, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Add helpers for direct write to stderr to consolidate the code
> and avoid the unused result warning in build.
> 
> v2: Use igt_ignore_warn since it is questionable whether igt_assert
>      is safe in signal handlers. (Chris Wilson)
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>   lib/igt_core.c | 23 ++++++++++++++++-------
>   1 file changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/lib/igt_core.c b/lib/igt_core.c
> index 538a4472e209..e866bf995872 100644
> --- a/lib/igt_core.c
> +++ b/lib/igt_core.c
> @@ -1239,7 +1239,7 @@ static const char hex[] = "0123456789abcdef";
>   static void
>   xputch(int c)
>   {
> -	write(STDERR_FILENO, (const void *) &c, 1);
> +	igt_ignore_warn(write(STDERR_FILENO, (const void *) &c, 1));
>   }
>   
>   static int
> @@ -1384,13 +1384,23 @@ xprintf(const char *fmt, ...)
>   	va_end(ap);
>   }
>   
> +static void __write_stderr(const char *str, size_t len)
> +{
> +	igt_ignore_warn(write(STDERR_FILENO, str, len));
> +}
> +
> +static void write_stderr(const char *str)
> +{
> +	__write_stderr(str, strlen(str));
> +}
> +
>   static void print_backtrace_sig_safe(void)
>   {
>   	unw_cursor_t cursor;
>   	unw_context_t uc;
>   	int stack_num = 0;
>   
> -	write(STDERR_FILENO, "Stack trace: \n", 15);
> +	write_stderr("Stack trace: \n");
>   
>   	unw_getcontext(&uc);
>   	unw_init_local(&cursor, &uc);
> @@ -1899,11 +1909,10 @@ static void fatal_sig_handler(int sig)
>   			continue;
>   
>   		if (handled_signals[i].name_len) {
> -			igt_assert_eq(write(STDERR_FILENO, "Received signal ", 16),
> -                                      16);
> -			igt_assert_eq(write(STDERR_FILENO, handled_signals[i].name, handled_signals[i].name_len),
> -                                      handled_signals[i].name_len);
> -			igt_assert_eq(write(STDERR_FILENO, ".\n", 2), 2);
> +			write_stderr("Received signal ");
> +			__write_stderr(handled_signals[i].name,
> +				       handled_signals[i].name_len);
> +			write_stderr(".\n");
>   		}
>   
>   		if (crash_signal(sig)) {
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t 3/5] lib/core: Avoid unused result in backtrace printing

[Chris Wilson]

Quoting Tvrtko Ursulin (2017-11-22 17:44:23)
> 
> Ping on this one - do we want to squash these compile warnings, yes? :)

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
 
> On 08/11/2017 12:06, Tvrtko Ursulin wrote:
> > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > 
> > Add helpers for direct write to stderr to consolidate the code
> > and avoid the unused result warning in build.
> > 
> > v2: Use igt_ignore_warn since it is questionable whether igt_assert
> >      is safe in signal handlers. (Chris Wilson)
> > 
> > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > ---
> >   lib/igt_core.c | 23 ++++++++++++++++-------
> >   1 file changed, 16 insertions(+), 7 deletions(-)
> > 
> > diff --git a/lib/igt_core.c b/lib/igt_core.c
> > index 538a4472e209..e866bf995872 100644
> > --- a/lib/igt_core.c
> > +++ b/lib/igt_core.c
> > @@ -1239,7 +1239,7 @@ static const char hex[] = "0123456789abcdef";
> >   static void
> >   xputch(int c)
> >   {
> > -     write(STDERR_FILENO, (const void *) &c, 1);
> > +     igt_ignore_warn(write(STDERR_FILENO, (const void *) &c, 1));
> >   }
> >   
> >   static int
> > @@ -1384,13 +1384,23 @@ xprintf(const char *fmt, ...)
> >       va_end(ap);
> >   }
> >   
> > +static void __write_stderr(const char *str, size_t len)
> > +{
> > +     igt_ignore_warn(write(STDERR_FILENO, str, len));
> > +}
> > +
> > +static void write_stderr(const char *str)
> > +{
> > +     __write_stderr(str, strlen(str));
> > +}
> > +
> >   static void print_backtrace_sig_safe(void)
> >   {
> >       unw_cursor_t cursor;
> >       unw_context_t uc;
> >       int stack_num = 0;
> >   
> > -     write(STDERR_FILENO, "Stack trace: \n", 15);
> > +     write_stderr("Stack trace: \n");
> >   
> >       unw_getcontext(&uc);
> >       unw_init_local(&cursor, &uc);
> > @@ -1899,11 +1909,10 @@ static void fatal_sig_handler(int sig)
> >                       continue;
> >   
> >               if (handled_signals[i].name_len) {
> > -                     igt_assert_eq(write(STDERR_FILENO, "Received signal ", 16),
> > -                                      16);
> > -                     igt_assert_eq(write(STDERR_FILENO, handled_signals[i].name, handled_signals[i].name_len),
> > -                                      handled_signals[i].name_len);
> > -                     igt_assert_eq(write(STDERR_FILENO, ".\n", 2), 2);
> > +                     write_stderr("Received signal ");
> > +                     __write_stderr(handled_signals[i].name,
> > +                                    handled_signals[i].name_len);

You could argue that we can kill name_len now that write_stderr makes it
tidier to do without.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t 4/5] gem_syslatency: Avoid arithmetic on void * warning

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 benchmarks/gem_syslatency.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/benchmarks/gem_syslatency.c b/benchmarks/gem_syslatency.c
index e2b1f74a43f4..580edc5f5c72 100644
--- a/benchmarks/gem_syslatency.c
+++ b/benchmarks/gem_syslatency.c
@@ -206,7 +206,7 @@ static void *sys_thp_alloc(void *arg)
 		assert(ptr != MAP_FAILED);
 		madvise(ptr, sz, MADV_HUGEPAGE);
 		for (size_t page = 0; page < sz; page += PAGE_SIZE)
-			*(volatile uint32_t *)(ptr + page) = 0;
+			*(volatile uint32_t *)((unsigned char *)ptr + page) = 0;
 		munmap(ptr, sz);
 
 		clock_gettime(CLOCK_MONOTONIC, &now);
-- 
2.14.1

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

Re: [PATCH i-g-t 4/5] gem_syslatency: Avoid arithmetic on void * warning

[Petri Latvala]

On Wed, Nov 08, 2017 at 12:06:53PM +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Chris wanted to disable the warning here instead. Either way is fine
with me so

Reviewed-by: Petri Latvala <petri.latvala@intel.com>


> ---
>  benchmarks/gem_syslatency.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/benchmarks/gem_syslatency.c b/benchmarks/gem_syslatency.c
> index e2b1f74a43f4..580edc5f5c72 100644
> --- a/benchmarks/gem_syslatency.c
> +++ b/benchmarks/gem_syslatency.c
> @@ -206,7 +206,7 @@ static void *sys_thp_alloc(void *arg)
>  		assert(ptr != MAP_FAILED);
>  		madvise(ptr, sz, MADV_HUGEPAGE);
>  		for (size_t page = 0; page < sz; page += PAGE_SIZE)
> -			*(volatile uint32_t *)(ptr + page) = 0;
> +			*(volatile uint32_t *)((unsigned char *)ptr + page) = 0;
>  		munmap(ptr, sz);
>  
>  		clock_gettime(CLOCK_MONOTONIC, &now);
> -- 
> 2.14.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t 4/5] gem_syslatency: Avoid arithmetic on void * warning

[Tvrtko Ursulin]

On 08/11/2017 12:28, Petri Latvala wrote:
> On Wed, Nov 08, 2017 at 12:06:53PM +0000, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Chris wanted to disable the warning here instead. Either way is fine
> with me so
> 
> Reviewed-by: Petri Latvala <petri.latvala@intel.com>

Thanks, I did not want to go into discussion of changing the build 
environment for the entire project. Or even just this file, while just 
getting rid build warnings.

Regards,

Tvrtko


> 
> 
>> ---
>>   benchmarks/gem_syslatency.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/benchmarks/gem_syslatency.c b/benchmarks/gem_syslatency.c
>> index e2b1f74a43f4..580edc5f5c72 100644
>> --- a/benchmarks/gem_syslatency.c
>> +++ b/benchmarks/gem_syslatency.c
>> @@ -206,7 +206,7 @@ static void *sys_thp_alloc(void *arg)
>>   		assert(ptr != MAP_FAILED);
>>   		madvise(ptr, sz, MADV_HUGEPAGE);
>>   		for (size_t page = 0; page < sz; page += PAGE_SIZE)
>> -			*(volatile uint32_t *)(ptr + page) = 0;
>> +			*(volatile uint32_t *)((unsigned char *)ptr + page) = 0;
>>   		munmap(ptr, sz);
>>   
>>   		clock_gettime(CLOCK_MONOTONIC, &now);
>> -- 
>> 2.14.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t 5/5] intel_gvtg_test: Handle system(3) return value.

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Avoid the build warning by checking the pkill either did not find
any guests or managed to send a signal to all of them.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 tools/intel_gvtg_test.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/intel_gvtg_test.c b/tools/intel_gvtg_test.c
index 7a29fbdde7cd..8c3d10cbd97f 100644
--- a/tools/intel_gvtg_test.c
+++ b/tools/intel_gvtg_test.c
@@ -144,7 +144,9 @@ static void create_guest(void)
 
 static void destroy_all_guest(void)
 {
-    system("pkill qemu");
+	int ret = system("pkill qemu");
+
+	igt_assert(ret == 0 || ret == 1);
 }
 
 static void remove_vgpu(void)
-- 
2.14.1

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

Re: [PATCH i-g-t 5/5] intel_gvtg_test: Handle system(3) return value.

[Chris Wilson]

Quoting Tvrtko Ursulin (2017-11-08 12:06:54)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Avoid the build warning by checking the pkill either did not find
> any guests or managed to send a signal to all of them.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  tools/intel_gvtg_test.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/intel_gvtg_test.c b/tools/intel_gvtg_test.c
> index 7a29fbdde7cd..8c3d10cbd97f 100644
> --- a/tools/intel_gvtg_test.c
> +++ b/tools/intel_gvtg_test.c
> @@ -144,7 +144,9 @@ static void create_guest(void)
>  
>  static void destroy_all_guest(void)
>  {
> -    system("pkill qemu");
> +       int ret = system("pkill qemu");
> +
> +       igt_assert(ret == 0 || ret == 1);

igt_terminate_process("qemu");

Still wants the return checked.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t 5/5] intel_gvtg_test: Handle system(3) return value.

[Tvrtko Ursulin]

On 08/11/2017 12:19, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2017-11-08 12:06:54)
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> Avoid the build warning by checking the pkill either did not find
>> any guests or managed to send a signal to all of them.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> ---
>>   tools/intel_gvtg_test.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/intel_gvtg_test.c b/tools/intel_gvtg_test.c
>> index 7a29fbdde7cd..8c3d10cbd97f 100644
>> --- a/tools/intel_gvtg_test.c
>> +++ b/tools/intel_gvtg_test.c
>> @@ -144,7 +144,9 @@ static void create_guest(void)
>>   
>>   static void destroy_all_guest(void)
>>   {
>> -    system("pkill qemu");
>> +       int ret = system("pkill qemu");
>> +
>> +       igt_assert(ret == 0 || ret == 1);
> 
> igt_terminate_process("qemu");
> 
> Still wants the return checked.

Opted not to go for the helper because:

a) It would change the behaviour (exact match vs not) which I don't want 
to go into.

b) igt_terminate_process looks inconsistent in return codes between its 
two implementations.

Regards,

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

Re: [PATCH i-g-t 5/5] intel_gvtg_test: Handle system(3) return value.

[Petri Latvala]

On Wed, Nov 08, 2017 at 12:06:54PM +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Avoid the build warning by checking the pkill either did not find
> any guests or managed to send a signal to all of them.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  tools/intel_gvtg_test.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/intel_gvtg_test.c b/tools/intel_gvtg_test.c
> index 7a29fbdde7cd..8c3d10cbd97f 100644
> --- a/tools/intel_gvtg_test.c
> +++ b/tools/intel_gvtg_test.c
> @@ -144,7 +144,9 @@ static void create_guest(void)
>  
>  static void destroy_all_guest(void)
>  {
> -    system("pkill qemu");
> +	int ret = system("pkill qemu");
> +
> +	igt_assert(ret == 0 || ret == 1);


system() returns a wait status and you need to inspect it with
WEXITSTATUS(ret).


-- 
Petri Latvala




>  }
>  
>  static void remove_vgpu(void)
> -- 
> 2.14.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t v2] intel_gvtg_test: Handle system(3) return value.

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Avoid the build warning by checking the pkill either did not find
any guests or managed to send a signal to all of them.

v2: Inspect system(3) return value properly. (Petri Latvala)

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 tools/intel_gvtg_test.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/intel_gvtg_test.c b/tools/intel_gvtg_test.c
index 7a29fbdde7cd..659b7956725c 100644
--- a/tools/intel_gvtg_test.c
+++ b/tools/intel_gvtg_test.c
@@ -144,7 +144,10 @@ static void create_guest(void)
 
 static void destroy_all_guest(void)
 {
-    system("pkill qemu");
+	int ret = system("pkill qemu");
+
+	igt_assert(ret >= 0 && WIFEXITED(ret));
+	igt_assert(WEXITSTATUS(ret) == 0 || WEXITSTATUS(ret) == 1);
 }
 
 static void remove_vgpu(void)
-- 
2.14.1

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

Re: [PATCH i-g-t v2] intel_gvtg_test: Handle system(3) return value.

[Petri Latvala]

On Wed, Nov 08, 2017 at 12:47:43PM +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Avoid the build warning by checking the pkill either did not find
> any guests or managed to send a signal to all of them.
> 
> v2: Inspect system(3) return value properly. (Petri Latvala)
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Reviewed-by: Petri Latvala <petri.latvala@intel.com>

Someone(tm) can follow this up with a patch that changes it to use
igt_terminate_process...


> ---
>  tools/intel_gvtg_test.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/intel_gvtg_test.c b/tools/intel_gvtg_test.c
> index 7a29fbdde7cd..659b7956725c 100644
> --- a/tools/intel_gvtg_test.c
> +++ b/tools/intel_gvtg_test.c
> @@ -144,7 +144,10 @@ static void create_guest(void)
>  
>  static void destroy_all_guest(void)
>  {
> -    system("pkill qemu");
> +	int ret = system("pkill qemu");
> +
> +	igt_assert(ret >= 0 && WIFEXITED(ret));
> +	igt_assert(WEXITSTATUS(ret) == 0 || WEXITSTATUS(ret) == 1);
>  }
>  
>  static void remove_vgpu(void)
> -- 
> 2.14.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t v2] intel_gvtg_test: Handle system(3) return value.

[Tvrtko Ursulin]

On 08/11/2017 12:52, Petri Latvala wrote:
> On Wed, Nov 08, 2017 at 12:47:43PM +0000, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> Avoid the build warning by checking the pkill either did not find
>> any guests or managed to send a signal to all of them.
>>
>> v2: Inspect system(3) return value properly. (Petri Latvala)
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Reviewed-by: Petri Latvala <petri.latvala@intel.com>
> 
> Someone(tm) can follow this up with a patch that changes it to use
> igt_terminate_process...

Better suited to actual owners of this file I think. I was only 
distracted by the compile warning. Since it is a bigger task  - question 
of case comparison, full vs substring, mismatching return codes between 
two implementations of igt_terminate_process...

Regards,

Tvrtko

> 
>> ---
>>   tools/intel_gvtg_test.c | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/intel_gvtg_test.c b/tools/intel_gvtg_test.c
>> index 7a29fbdde7cd..659b7956725c 100644
>> --- a/tools/intel_gvtg_test.c
>> +++ b/tools/intel_gvtg_test.c
>> @@ -144,7 +144,10 @@ static void create_guest(void)
>>   
>>   static void destroy_all_guest(void)
>>   {
>> -    system("pkill qemu");
>> +	int ret = system("pkill qemu");
>> +
>> +	igt_assert(ret >= 0 && WIFEXITED(ret));
>> +	igt_assert(WEXITSTATUS(ret) == 0 || WEXITSTATUS(ret) == 1);
>>   }
>>   
>>   static void remove_vgpu(void)
>> -- 
>> 2.14.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: warning for intel_gvtg_test: Handle system(3) return value.

[Patchwork]

== Series Details ==

Series: intel_gvtg_test: Handle system(3) return value.
URL   : https://patchwork.freedesktop.org/series/33422/
State : warning

== Summary ==

IGT patchset tested on top of latest successful build
9fe5a9a3de9e2ce345d5967a1e10a9a586b19836 tests/kms_fbcon_fbt: Report fbc_status on error

with latest DRM-Tip kernel build CI_DRM_3321
087c404bd6d5 drm-tip: 2017y-11m-08d-15h-44m-06s UTC integration manifest

No testlist changes.

Test chamelium:
        Subgroup dp-crc-fast:
                pass       -> FAIL       (fi-kbl-7500u) fdo#102514
Test gem_exec_parallel:
        Subgroup basic:
                pass       -> DMESG-WARN (fi-bsw-n3050)

fdo#102514 https://bugs.freedesktop.org/show_bug.cgi?id=102514

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:446s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:459s
fi-blb-e6850     total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:384s
fi-bsw-n3050     total:289  pass:242  dwarn:1   dfail:0   fail:0   skip:46  time:540s
fi-bwr-2160      total:289  pass:183  dwarn:0   dfail:0   fail:0   skip:106 time:274s
fi-bxt-dsi       total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  time:503s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:511s
fi-byt-j1900     total:289  pass:254  dwarn:0   dfail:0   fail:0   skip:35  time:497s
fi-byt-n2820     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:488s
fi-elk-e7500     total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:433s
fi-gdg-551       total:289  pass:178  dwarn:1   dfail:0   fail:1   skip:109 time:265s
fi-glk-1         total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:541s
fi-hsw-4770      total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:432s
fi-hsw-4770r     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:442s
fi-ilk-650       total:289  pass:228  dwarn:0   dfail:0   fail:0   skip:61  time:429s
fi-ivb-3520m     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:480s
fi-ivb-3770      total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:461s
fi-kbl-7500u     total:289  pass:263  dwarn:1   dfail:0   fail:1   skip:24  time:479s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:520s
fi-kbl-7567u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:476s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:533s
fi-pnv-d510      total:289  pass:222  dwarn:1   dfail:0   fail:0   skip:66  time:568s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:465s
fi-skl-6600u     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:542s
fi-skl-6700hq    total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:560s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:528s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:498s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:463s
fi-snb-2520m     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:561s
fi-snb-2600      total:289  pass:249  dwarn:0   dfail:0   fail:0   skip:40  time:421s
Blacklisted hosts:
fi-cnl-y         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:568s
fi-glk-dsi       total:289  pass:258  dwarn:0   dfail:0   fail:1   skip:30  time:492s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_483/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.IGT: warning for intel_gvtg_test: Handle system(3) return value.

[Patchwork]

== Series Details ==

Series: intel_gvtg_test: Handle system(3) return value.
URL   : https://patchwork.freedesktop.org/series/33422/
State : warning

== Summary ==

Test perf:
        Subgroup blocking:
                fail       -> PASS       (shard-hsw) fdo#102252 +1
Test kms_busy:
        Subgroup extended-modeset-hang-newfb-with-reset-render-a:
                dmesg-warn -> PASS       (shard-hsw) fdo#102249
Test kms_setmode:
        Subgroup basic:
                fail       -> PASS       (shard-hsw) fdo#99912
Test drv_module_reload:
        Subgroup basic-reload-inject:
                pass       -> DMESG-WARN (shard-hsw) fdo#102707
Test kms_plane_multiple:
        Subgroup atomic-pipe-a-tiling-x:
                pass       -> SKIP       (shard-hsw)
Test kms_chv_cursor_fail:
        Subgroup pipe-a-256x256-top-edge:
                pass       -> SKIP       (shard-hsw)

fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
fdo#102249 https://bugs.freedesktop.org/show_bug.cgi?id=102249
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
fdo#102707 https://bugs.freedesktop.org/show_bug.cgi?id=102707

shard-hsw        total:2540 pass:1431 dwarn:1   dfail:0   fail:9   skip:1099 time:9161s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_483/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for Assorted compile warnings fixes

[Patchwork]

== Series Details ==

Series: Assorted compile warnings fixes
URL   : https://patchwork.freedesktop.org/series/33417/
State : failure

== Summary ==

IGT patchset build failed on latest successful build
9fe5a9a3de9e2ce345d5967a1e10a9a586b19836 tests/kms_fbcon_fbt: Report fbc_status on error

make  all-recursive
Making all in lib
make  all-recursive
Making all in .
Making all in tests
make[4]: Nothing to be done for 'all'.
Making all in man
make[2]: Nothing to be done for 'all'.
Making all in tools
Making all in null_state_gen
make[3]: Nothing to be done for 'all'.
Making all in registers
make[3]: Nothing to be done for 'all'.
  CC       aubdump.lo
aubdump.c: In function ‘dump_execbuffer2’:
aubdump.c:465:1: error: version control conflict marker in file
 <<<<<<< HEAD
 ^~~~~~~
aubdump.c:467:1: error: version control conflict marker in file
 =======
 ^~~~~~~
aubdump.c:469:1: error: version control conflict marker in file
 >>>>>>> aubdump: Avoid mixing declarations and code
 ^~~~~~~
aubdump.c:410:6: warning: variable ‘batch_index’ set but not used [-Wunused-but-set-variable]
  int batch_index;
      ^~~~~~~~~~~
Makefile:1163: recipe for target 'aubdump.lo' failed
make[3]: *** [aubdump.lo] Error 1
Makefile:1211: recipe for target 'all-recursive' failed
make[2]: *** [all-recursive] Error 1
Makefile:533: recipe for target 'all-recursive' failed
make[1]: *** [all-recursive] Error 1
Makefile:465: recipe for target 'all' failed
make: *** [all] Error 2

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