| Message ID | fcdf5cfd9ed825fd090696a8014131cd249d2ed4.1531005542.git.rodrigosiqueiramelo@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 >
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
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.
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
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
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:
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(-)