[PATCH i-g-t 0/7] GCC warning cleanup

[PATCH i-g-t 0/7] GCC warning cleanup

[Rodrigo Siqueira]

This patchset fixes all the current GCC 8.1.1 warnings found in the IGT.

Rodrigo Siqueira (7):
  Remove parameter aliases with another argument
  Cast void * pointer used in arithmetic to uint32_t*
  Fix comparison that always evaluates to false
  Fix truncate string in the snprintf
  Make string commands dynamic allocate
  Fix truncate string in the strncpy
  Avoid truncate string in __igt_lsof_fds

 lib/igt_aux.c            |  2 +-
 lib/igt_core.c           |  6 +++---
 lib/igt_fb.c             |  6 ++++--
 tests/perf.c             | 10 +++++-----
 tools/intel_gvtg_test.c  | 27 ++++++++++++++++++---------
 tools/intel_reg.c        |  2 +-
 tools/intel_reg_decode.c |  2 +-
 7 files changed, 33 insertions(+), 22 deletions(-)

-- 
2.18.0

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

[PATCH i-g-t 1/7] Remove parameter aliases with another argument

[Rodrigo Siqueira]

This commit fixes the following GCC warning:

warning: passing argument 2 to restrict-qualified parameter aliases with
argument 1 [-Wrestrict]
  return (readlink (buf, buf, sizeof (buf)) != -1 &&

This commit fixes the GCC warning by creating a second buffer only to
keep the path.

Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
---
 lib/igt_core.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/igt_core.c b/lib/igt_core.c
index 3313050c..fa22f12d 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -1169,10 +1169,10 @@ bool igt_can_fail(void)
 
 static bool run_under_gdb(void)
 {
-	char buf[1024];
+	char pathname[1024], buf[1024];
 
-	sprintf(buf, "/proc/%d/exe", getppid());
-	return (readlink (buf, buf, sizeof (buf)) != -1 &&
+	sprintf(pathname, "/proc/%d/exe", getppid());
+	return (readlink (pathname, buf, sizeof (buf)) != -1 &&
 		strncmp(basename(buf), "gdb", 3) == 0);
 }
 
-- 
2.18.0

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

[PATCH i-g-t 2/7] Cast void * pointer used in arithmetic to uint32_t*

[Rodrigo Siqueira]

This commit fixes the GCC warning:

warning: pointer of type ‘void *’ used in arithmetic [-Wpointer-arith]
     memset(ptr + offsets[0], full_range ? 0x00 : 0x10,
                ^
warning: pointer of type ‘void *’ used in arithmetic [-Wpointer-arith]
     memset(ptr + offsets[1], 0x80,

This commit cast the ptr pointer, which is a void *, to uint32_t * in
the pointer arithmetic operation.

Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
---
 lib/igt_fb.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index ae71d967..ca905038 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -410,9 +410,11 @@ static int create_bo_for_fb(int fd, int width, int height,
 
 			switch (format->drm_id) {
 			case DRM_FORMAT_NV12:
-				memset(ptr + offsets[0], full_range ? 0x00 : 0x10,
+				memset(((uint32_t *)ptr) + offsets[0],
+				       full_range ? 0x00 : 0x10,
 				       calculated_stride * height);
-				memset(ptr + offsets[1], 0x80,
+				memset(((uint32_t *)ptr) + offsets[1],
+				       0x80,
 				       calculated_stride * height/2);
 				break;
 			case DRM_FORMAT_YUYV:
-- 
2.18.0

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

[PATCH i-g-t 3/7] Fix comparison that always evaluates to false

[Rodrigo Siqueira]

This commit fix the GCC warning:

warning: bitwise comparison always evaluates to false
[-Wtautological-compare]
  } else if ((val & DPLLB_MODE_LVDS) == DPLLB_MODE_DAC_SERIAL) {

The first comparison already checks DPLLB_MODE_LVDS, in this sense, the
second 'if' condition always will be false. This commit changes the
comparison to DPLLB_MODE_DAC_SERIAL.

Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
---
 tools/intel_reg_decode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/intel_reg_decode.c b/tools/intel_reg_decode.c
index f3c7d74a..5a632e09 100644
--- a/tools/intel_reg_decode.c
+++ b/tools/intel_reg_decode.c
@@ -1277,7 +1277,7 @@ DEBUGSTRING(ironlake_debug_pch_dpll)
 			p2 = "Div 7";
 		else
 			p2 = "Div 14";
-	} else if ((val & DPLLB_MODE_LVDS) == DPLLB_MODE_DAC_SERIAL) {
+	} else if ((val & DPLLB_MODE_DAC_SERIAL) == DPLLB_MODE_DAC_SERIAL) {
 		mode = "Non-LVDS";
 		if (val & DPLL_DAC_SERIAL_P2_CLOCK_DIV_5)
 			p2 = "Div 5";
-- 
2.18.0

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

[PATCH i-g-t 4/7] Fix truncate string in the snprintf

[Rodrigo Siqueira]

This patch fix the following GCC warning:

intel_reg.c: In function ‘dump_decode’:
warning: ‘snprintf’ output may be truncated before the last format character [-Wformat-truncation=]
   snprintf(decode, sizeof(decode), "\n%s", bin);
intel_reg.c:203:3: note: ‘snprintf’ output between 2 and 1025 bytes into a destination of size 1024
[..]

Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
---
 tools/intel_reg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/intel_reg.c b/tools/intel_reg.c
index ddff2794..15ebb86a 100644
--- a/tools/intel_reg.c
+++ b/tools/intel_reg.c
@@ -180,7 +180,7 @@ static void to_binary(char *buf, size_t buflen, uint32_t val)
 
 static void dump_decode(struct config *config, struct reg *reg, uint32_t val)
 {
-	char decode[1024];
+	char decode[2060];
 	char tmp[1024];
 	char bin[1024];
 
-- 
2.18.0

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

[PATCH i-g-t 5/7] Make string commands dynamic allocate

[Rodrigo Siqueira]

This patch fix the following GCC warning:

intel_gvtg_test.c: In function ‘create_guest’:
intel_gvtg_test.c:127:50: warning: ‘%s’ directive writing up to 4095
bytes into a region of size 4077 [-Wformat-overflow=]
[..]
intel_gvtg_test.c:127:5: note: ‘sprintf’ output between 36 and 8226
bytes into a destination of size 4096
[..]

This patch changes the approach for allocating memory to handle QEMU
commands by dynamically allocate space to save the whole command.

Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
---
 tools/intel_gvtg_test.c | 27 ++++++++++++++++++---------
 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/tools/intel_gvtg_test.c b/tools/intel_gvtg_test.c
index 659b7956..93c05e37 100644
--- a/tools/intel_gvtg_test.c
+++ b/tools/intel_gvtg_test.c
@@ -120,16 +120,25 @@ static int check_tools(void)
 
 static void create_guest(void)
 {
-    char create_qcow_cmd[PATH_MAX] = {0};
-    char create_vgpu_cmd[PATH_MAX] = {0};
-    char create_instance_cmd[PATH_MAX] = {0};
+    unsigned int max_size_cmd = sysconf(_SC_ARG_MAX);
+    char *command;
 
-    sprintf(create_qcow_cmd, "qemu-img create -b %s -f qcow2 %s.qcow2",
+    command = malloc(max_size_cmd);
+    if (!command)
+        return;
+
+    sprintf(command, "qemu-img create -b %s -f qcow2 %s.qcow2",
             hda_path, hda_path);
-    sprintf(create_vgpu_cmd, "echo \"%s\" > /sys/bus/pci/devices/0000:00:02.0/"
+    igt_assert_eq(system(command), 0);
+    memset(command, 0, max_size_cmd);
+
+    sprintf(command, "echo \"%s\" > /sys/bus/pci/devices/0000:00:02.0/"
            "mdev_supported_types/$(ls /sys/bus/pci/devices/0000:00:02.0/"
            "mdev_supported_types |awk {'print $1'}|tail -1)/create", uuid);
-    sprintf(create_instance_cmd, "%s -m 2048 -smp 2 -M pc -name gvtg_guest"
+    igt_assert_eq(system(command), 0);
+    memset(command, 0, max_size_cmd);
+
+    sprintf(command, "%s -m 2048 -smp 2 -M pc -name gvtg_guest"
            " -hda %s.qcow2 -bios %s -enable-kvm --net nic,macaddr=%s -net"
            " tap,script=/etc/qemu-ifup -vga cirrus -k en-us"
            " -serial stdio -vnc :1 -machine kernel_irqchip=on -global"
@@ -137,9 +146,9 @@ static void create_guest(void)
            " -usb -usbdevice tablet -device vfio-pci,sysfsdev="
            "/sys/bus/pci/devices/0000:00:02.0/%s &",
            qemu_path, hda_path, bios_path, mac_addr, uuid);
-    igt_assert_eq(system(create_qcow_cmd), 0);
-    igt_assert_eq(system(create_vgpu_cmd), 0);
-    igt_assert_eq(system(create_instance_cmd), 0);
+    igt_assert_eq(system(command), 0);
+
+    free(command);
 }
 
 static void destroy_all_guest(void)
-- 
2.18.0

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

[PATCH i-g-t 6/7] Fix truncate string in the strncpy

[Rodrigo Siqueira]

This patch fix the following GCC Warnings:

warning: ‘strncpy’ output truncated before terminating nul copying 36
bytes from a string of the same length [-Wstringop-truncation]
[..]

Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
---
 tests/perf.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tests/perf.c b/tests/perf.c
index 95048bfa..031bcee8 100644
--- a/tests/perf.c
+++ b/tests/perf.c
@@ -3693,7 +3693,7 @@ test_invalid_create_userspace_config(void)
 	igt_assert_eq(__i915_perf_add_config(drm_fd, &config), -EINVAL);
 
 	/* invalid mux_regs */
-	strncpy(config.uuid, uuid, sizeof(config.uuid));
+	strncpy(config.uuid, uuid, strlen(config.uuid));
 	config.n_mux_regs = 1;
 	config.mux_regs_ptr = to_user_pointer(invalid_mux_regs);
 	config.n_boolean_regs = 0;
@@ -3702,7 +3702,7 @@ test_invalid_create_userspace_config(void)
 	igt_assert_eq(__i915_perf_add_config(drm_fd, &config), -EINVAL);
 
 	/* empty config */
-	strncpy(config.uuid, uuid, sizeof(config.uuid));
+	strncpy(config.uuid, uuid, strlen(config.uuid));
 	config.n_mux_regs = 0;
 	config.mux_regs_ptr = to_user_pointer(mux_regs);
 	config.n_boolean_regs = 0;
@@ -3711,7 +3711,7 @@ test_invalid_create_userspace_config(void)
 	igt_assert_eq(__i915_perf_add_config(drm_fd, &config), -EINVAL);
 
 	/* empty config with null pointers */
-	strncpy(config.uuid, uuid, sizeof(config.uuid));
+	strncpy(config.uuid, uuid, strlen(config.uuid));
 	config.n_mux_regs = 1;
 	config.mux_regs_ptr = to_user_pointer(NULL);
 	config.n_boolean_regs = 2;
@@ -3722,7 +3722,7 @@ test_invalid_create_userspace_config(void)
 	igt_assert_eq(__i915_perf_add_config(drm_fd, &config), -EINVAL);
 
 	/* invalid pointers */
-	strncpy(config.uuid, uuid, sizeof(config.uuid));
+	strncpy(config.uuid, uuid, strlen(config.uuid));
 	config.n_mux_regs = 42;
 	config.mux_regs_ptr = to_user_pointer((void *) 0xDEADBEEF);
 	config.n_boolean_regs = 0;
@@ -3809,7 +3809,7 @@ test_create_destroy_userspace_config(void)
 		i915_perf_remove_config(drm_fd, config_id);
 
 	memset(&config, 0, sizeof(config));
-	strncpy(config.uuid, uuid, sizeof(config.uuid));
+	strncpy(config.uuid, uuid, strlen(config.uuid));
 
 	config.n_mux_regs = 1;
 	config.mux_regs_ptr = to_user_pointer(mux_regs);
-- 
2.18.0

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

[PATCH i-g-t 7/7] Avoid truncate string in __igt_lsof_fds

[Rodrigo Siqueira]

Note that 'proc_path' parameter in __igt_lsof_fds receives a string
which was initialized with the size of PATH_MAX and the local variable
'path' has the same size, but it also have to append: '/', '\0', and the
directory name. This situation caused the warning described below.

warning: ‘%s’ directive output may be truncated writing up to 255 bytes
into a region of size between 0 and 4095 [-Wformat-truncation=]
snprintf(path, sizeof(path), "%s/%s", proc_path, d->d_name);
note: ‘snprintf’ output between 2 and 4352 bytes into a destination of
size 4096 [..]

This commit fixes this problem by changing the string size passed by
__igt_lsoft to __igt_lsof_fds. The max size for the string is
strlen("/proc/%d/cwd")+1 where "%d" can be estimated with
CEILING(LOG_10(INT_MAX)), in this sense, it is safe to define a path
size of 30 characters.

Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
---
 lib/igt_aux.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/igt_aux.c b/lib/igt_aux.c
index ca8ccfbd..d9dbf7ce 100644
--- a/lib/igt_aux.c
+++ b/lib/igt_aux.c
@@ -1485,7 +1485,7 @@ __igt_lsof(const char *dir)
 	PROCTAB *proc;
 	proc_t *proc_info;
 
-	char path[PATH_MAX];
+	char path[30];
 	char *name_lnk;
 	struct stat st;
 	int state = 0;
-- 
2.18.0

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

Re: [PATCH i-g-t 1/7] Remove parameter aliases with another argument

[Arkadiusz Hiler]

On Sat, Jul 07, 2018 at 08:22:25PM -0300, Rodrigo Siqueira wrote:
> This commit fixes the following GCC warning:
> 
> warning: passing argument 2 to restrict-qualified parameter aliases with
> argument 1 [-Wrestrict]
>   return (readlink (buf, buf, sizeof (buf)) != -1 &&
> 
> This commit fixes the GCC warning by creating a second buffer only to
> keep the path.
> 
> Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
> ---
>  lib/igt_core.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/igt_core.c b/lib/igt_core.c
> index 3313050c..fa22f12d 100644
> --- a/lib/igt_core.c
> +++ b/lib/igt_core.c
> @@ -1169,10 +1169,10 @@ bool igt_can_fail(void)
>  
>  static bool run_under_gdb(void)
>  {
> -	char buf[1024];
> +	char pathname[1024], buf[1024];

1024 for pathname is quite an overshoot. We stash at most
"/proc/%d/exec" where %d should be at most 21 or so.

> -	sprintf(buf, "/proc/%d/exe", getppid());
> -	return (readlink (buf, buf, sizeof (buf)) != -1 &&
> +	sprintf(pathname, "/proc/%d/exe", getppid());
> +	return (readlink (pathname, buf, sizeof (buf)) != -1 &&

                        ^ while we are here we can get rid of that space

>  		strncmp(basename(buf), "gdb", 3) == 0);
>  }
>  
> -- 
> 2.18.0
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t 2/7] Cast void * pointer used in arithmetic to uint32_t*

[Arkadiusz Hiler]

On Sat, Jul 07, 2018 at 08:22:38PM -0300, Rodrigo Siqueira wrote:
> This commit fixes the GCC warning:
> 
> warning: pointer of type ‘void *’ used in arithmetic [-Wpointer-arith]
>      memset(ptr + offsets[0], full_range ? 0x00 : 0x10,
>                 ^
> warning: pointer of type ‘void *’ used in arithmetic [-Wpointer-arith]
>      memset(ptr + offsets[1], 0x80,
> 
> This commit cast the ptr pointer, which is a void *, to uint32_t * in
> the pointer arithmetic operation.

This will change semantics, as according to GNU C standard[1], void
pointers have a size of 1 for all arithmetical purposes.

So you should be using uint8_t (or char) pointer instead.

[1]: http://gcc.gnu.org/onlinedocs/gcc/Pointer-Arith.html

> Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
> ---
>  lib/igt_fb.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index ae71d967..ca905038 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -410,9 +410,11 @@ static int create_bo_for_fb(int fd, int width, int height,
>  
>  			switch (format->drm_id) {
>  			case DRM_FORMAT_NV12:
> -				memset(ptr + offsets[0], full_range ? 0x00 : 0x10,
> +				memset(((uint32_t *)ptr) + offsets[0],
> +				       full_range ? 0x00 : 0x10,
>  				       calculated_stride * height);
> -				memset(ptr + offsets[1], 0x80,
> +				memset(((uint32_t *)ptr) + offsets[1],
> +				       0x80,
>  				       calculated_stride * height/2);
>  				break;
>  			case DRM_FORMAT_YUYV:
> -- 
> 2.18.0
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t 4/7] Fix truncate string in the snprintf

[Arkadiusz Hiler]

On Sat, Jul 07, 2018 at 08:23:17PM -0300, Rodrigo Siqueira wrote:
> This patch fix the following GCC warning:
> 
> intel_reg.c: In function ‘dump_decode’:
> warning: ‘snprintf’ output may be truncated before the last format character [-Wformat-truncation=]
>    snprintf(decode, sizeof(decode), "\n%s", bin);
> intel_reg.c:203:3: note: ‘snprintf’ output between 2 and 1025 bytes into a destination of size 1024
> [..]

That's an odd error.

Line 203:
	snprintf(decode, sizeof(decode), "\n%s", bin);

man snprintf states:
	The functions snprintf() and vsnprintf() write at most size
	bytes (including the terminating null byte ('\0')) to str.

So this should be truncated correctly. Am I missing something?

I'll set up gcc 8.1 tomorrow and poke around.

> Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
> ---
>  tools/intel_reg.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/intel_reg.c b/tools/intel_reg.c
> index ddff2794..15ebb86a 100644
> --- a/tools/intel_reg.c
> +++ b/tools/intel_reg.c
> @@ -180,7 +180,7 @@ static void to_binary(char *buf, size_t buflen, uint32_t val)
>  
>  static void dump_decode(struct config *config, struct reg *reg, uint32_t val)
>  {
> -	char decode[1024];
> +	char decode[2060];
>  	char tmp[1024];
>  	char bin[1024];
>  
> -- 
> 2.18.0
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [PATCH i-g-t 2/7] Cast void * pointer used in arithmetic to uint32_t*

[Chris Wilson]

Quoting Arkadiusz Hiler (2018-07-10 14:58:49)
> On Sat, Jul 07, 2018 at 08:22:38PM -0300, Rodrigo Siqueira wrote:
> > This commit fixes the GCC warning:
> > 
> > warning: pointer of type ‘void *’ used in arithmetic [-Wpointer-arith]
> >      memset(ptr + offsets[0], full_range ? 0x00 : 0x10,
> >                 ^
> > warning: pointer of type ‘void *’ used in arithmetic [-Wpointer-arith]
> >      memset(ptr + offsets[1], 0x80,
> > 
> > This commit cast the ptr pointer, which is a void *, to uint32_t * in
> > the pointer arithmetic operation.
> 
> This will change semantics, as according to GNU C standard[1], void
> pointers have a size of 1 for all arithmetical purposes.
> 
> So you should be using uint8_t (or char) pointer instead.

Please just fix the compiler flags, we want close compatibility with the
kernel coding standards which explicitly allow void arithmetic for the
simplicity it lends to writing code.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [PATCH i-g-t 2/7] Cast void * pointer used in arithmetic to uint32_t*

[Arkadiusz Hiler]

On Tue, Jul 10, 2018 at 03:09:25PM +0100, Chris Wilson wrote:
> Quoting Arkadiusz Hiler (2018-07-10 14:58:49)
> > On Sat, Jul 07, 2018 at 08:22:38PM -0300, Rodrigo Siqueira wrote:
> > > This commit fixes the GCC warning:
> > > 
> > > warning: pointer of type ‘void *’ used in arithmetic [-Wpointer-arith]
> > >      memset(ptr + offsets[0], full_range ? 0x00 : 0x10,
> > >                 ^
> > > warning: pointer of type ‘void *’ used in arithmetic [-Wpointer-arith]
> > >      memset(ptr + offsets[1], 0x80,
> > > 
> > > This commit cast the ptr pointer, which is a void *, to uint32_t * in
> > > the pointer arithmetic operation.
> > 
> > This will change semantics, as according to GNU C standard[1], void
> > pointers have a size of 1 for all arithmetical purposes.
> > 
> > So you should be using uint8_t (or char) pointer instead.
> 
> Please just fix the compiler flags, we want close compatibility with the
> kernel coding standards which explicitly allow void arithmetic for the
> simplicity it lends to writing code.
> -Chris

Fair point.

We don't rise the error with meson, so it is not a change in the gcc
defaults. Somehow autotools manage to end up adding -Wpointer-arith to
BASE_CFLAGS.

I don't think we should invest time into making autotools behave, since
it's going to be dropped completely. Hopefully this will happen sooner
than later.

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

Re: [igt-dev] [PATCH i-g-t 2/7] Cast void * pointer used in arithmetic to uint32_t*

[Chris Wilson]

Quoting Arkadiusz Hiler (2018-07-10 15:38:26)
> On Tue, Jul 10, 2018 at 03:09:25PM +0100, Chris Wilson wrote:
> > Quoting Arkadiusz Hiler (2018-07-10 14:58:49)
> > > On Sat, Jul 07, 2018 at 08:22:38PM -0300, Rodrigo Siqueira wrote:
> > > > This commit fixes the GCC warning:
> > > > 
> > > > warning: pointer of type ‘void *’ used in arithmetic [-Wpointer-arith]
> > > >      memset(ptr + offsets[0], full_range ? 0x00 : 0x10,
> > > >                 ^
> > > > warning: pointer of type ‘void *’ used in arithmetic [-Wpointer-arith]
> > > >      memset(ptr + offsets[1], 0x80,
> > > > 
> > > > This commit cast the ptr pointer, which is a void *, to uint32_t * in
> > > > the pointer arithmetic operation.
> > > 
> > > This will change semantics, as according to GNU C standard[1], void
> > > pointers have a size of 1 for all arithmetical purposes.
> > > 
> > > So you should be using uint8_t (or char) pointer instead.
> > 
> > Please just fix the compiler flags, we want close compatibility with the
> > kernel coding standards which explicitly allow void arithmetic for the
> > simplicity it lends to writing code.
> > -Chris
> 
> Fair point.
> 
> We don't rise the error with meson, so it is not a change in the gcc
> defaults. Somehow autotools manage to end up adding -Wpointer-arith to
> BASE_CFLAGS.

iirc, it's pulled in from xorg-macros. Maybe just tack a
-Wnopointer-arith onto the end, or sed away.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [PATCH i-g-t 2/7] Cast void * pointer used in arithmetic to uint32_t*

[Rodrigo Siqueira]

Hi,

Thanks for the feedback,

I'm just a little bit confused about the next step here. Is it worth to
fix this patch and the others? I got confused since you said that IGT
will remove autotools and some of the warnings here does not appear on
meson.

Best Regards
Rodrigo Siqueira

On 07/10, Arkadiusz Hiler wrote:
> On Tue, Jul 10, 2018 at 03:09:25PM +0100, Chris Wilson wrote:
> > Quoting Arkadiusz Hiler (2018-07-10 14:58:49)
> > > On Sat, Jul 07, 2018 at 08:22:38PM -0300, Rodrigo Siqueira wrote:
> > > > This commit fixes the GCC warning:
> > > > 
> > > > warning: pointer of type ‘void *’ used in arithmetic [-Wpointer-arith]
> > > >      memset(ptr + offsets[0], full_range ? 0x00 : 0x10,
> > > >                 ^
> > > > warning: pointer of type ‘void *’ used in arithmetic [-Wpointer-arith]
> > > >      memset(ptr + offsets[1], 0x80,
> > > > 
> > > > This commit cast the ptr pointer, which is a void *, to uint32_t * in
> > > > the pointer arithmetic operation.
> > > 
> > > This will change semantics, as according to GNU C standard[1], void
> > > pointers have a size of 1 for all arithmetical purposes.
> > > 
> > > So you should be using uint8_t (or char) pointer instead.
> > 
> > Please just fix the compiler flags, we want close compatibility with the
> > kernel coding standards which explicitly allow void arithmetic for the
> > simplicity it lends to writing code.
> > -Chris
> 
> Fair point.
> 
> We don't rise the error with meson, so it is not a change in the gcc
> defaults. Somehow autotools manage to end up adding -Wpointer-arith to
> BASE_CFLAGS.
> 
> I don't think we should invest time into making autotools behave, since
> it's going to be dropped completely. Hopefully this will happen sooner
> than later.
> 
> -- 
> Cheers,
> Arek
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t 5/7] Make string commands dynamic allocate

[Arkadiusz Hiler]

On Sat, Jul 07, 2018 at 08:23:30PM -0300, Rodrigo Siqueira wrote:
> This patch fix the following GCC warning:
> 
> intel_gvtg_test.c: In function ‘create_guest’:
> intel_gvtg_test.c:127:50: warning: ‘%s’ directive writing up to 4095
> bytes into a region of size 4077 [-Wformat-overflow=]
> [..]
> intel_gvtg_test.c:127:5: note: ‘sprintf’ output between 36 and 8226
> bytes into a destination of size 4096
> [..]
> 
> This patch changes the approach for allocating memory to handle QEMU
> commands by dynamically allocate space to save the whole command.
> 
> Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
> ---
>  tools/intel_gvtg_test.c | 27 ++++++++++++++++++---------
>  1 file changed, 18 insertions(+), 9 deletions(-)
> 
> diff --git a/tools/intel_gvtg_test.c b/tools/intel_gvtg_test.c
> index 659b7956..93c05e37 100644
> --- a/tools/intel_gvtg_test.c
> +++ b/tools/intel_gvtg_test.c
> @@ -120,16 +120,25 @@ static int check_tools(void)
>  
>  static void create_guest(void)
>  {
> -    char create_qcow_cmd[PATH_MAX] = {0};
> -    char create_vgpu_cmd[PATH_MAX] = {0};
> -    char create_instance_cmd[PATH_MAX] = {0};
> +    unsigned int max_size_cmd = sysconf(_SC_ARG_MAX);

That's 2097152 on my system. Quite an overkill. I know that we have lazy
page table assigement, but memseting the whole 2GB defeats it.

The qemu invocation looks like  the longest one, and we have 3
components that may take up to PATH_MAX each and some extra stuff, so
going with 4*PATH_MAX should be enough.

BTW, why do you memset it to 0 before each sprintf?

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