| Message ID | 1452633260-26767-1-git-send-email-maraeo@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 12 January 2016 at 23:14, Marek Olšák <maraeo@gmail.com> wrote: > From: Marek Olšák <marek.olsak@amd.com> > > It warns for all "{}" initializers. Well, I want us to use {}. > --- > configure.ac | 3 ++- > intel/intel_decode.c | 2 -- The whole of libdrm, minus the intel_decode can get away without using such constructs. And yes that includes radeon and amdgpu. NACK on this one - please be consistent with existing code base. -Emil
On Fri, Jan 15, 2016 at 12:12 PM, Emil Velikov <emil.l.velikov@gmail.com> wrote: > On 12 January 2016 at 23:14, Marek Olšák <maraeo@gmail.com> wrote: >> From: Marek Olšák <marek.olsak@amd.com> >> >> It warns for all "{}" initializers. Well, I want us to use {}. >> --- >> configure.ac | 3 ++- >> intel/intel_decode.c | 2 -- > The whole of libdrm, minus the intel_decode can get away without using > such constructs. And yes that includes radeon and amdgpu. > > NACK on this one - please be consistent with existing code base. Consistent with what? {} is the same as memset on each structure member. The warning says that a structure member is initialized to zero because of {}, which is why {} is used in the first place. It's the same as using memset and getting a warning "memset initializes the memory to zero". How useful is that? libdrm does have a lot of optional warnings enabled. Mesa does not, and Mesa does not even have this one. This means libdrm is inconsistent with Mesa and, BTW, it's also inconsistent with the kernel. It looks like somebody enabled optional warnings for libdrm in the past. All I'm doing is aligning the behavior with Mesa/kernel, which is what we would like to have and so would Intel apparently. Do you still think we are inconsistent? Thanks, Marek
Hi On Fri, Jan 15, 2016 at 4:24 PM, Marek Olšák <maraeo@gmail.com> wrote: > On Fri, Jan 15, 2016 at 12:12 PM, Emil Velikov <emil.l.velikov@gmail.com> wrote: >> On 12 January 2016 at 23:14, Marek Olšák <maraeo@gmail.com> wrote: >>> From: Marek Olšák <marek.olsak@amd.com> >>> >>> It warns for all "{}" initializers. Well, I want us to use {}. >>> --- >>> configure.ac | 3 ++- >>> intel/intel_decode.c | 2 -- >> The whole of libdrm, minus the intel_decode can get away without using >> such constructs. And yes that includes radeon and amdgpu. >> >> NACK on this one - please be consistent with existing code base. > > Consistent with what? {} is the same as memset on each structure > member. The warning says that a structure member is initialized to > zero because of {}, which is why {} is used in the first place. It's > the same as using memset and getting a warning "memset initializes the > memory to zero". How useful is that? The only use of this warning is to prevent people from learning that {} initializes any non-specified field to 0. Reviewed-by: David Herrmann <dh.herrmann@gmail.com> Thanks David
Reviewed-by: Ilia Mirkin <imirkin@alum.mit.edu> On Tue, Jan 12, 2016 at 4:14 PM, Marek Olšák <maraeo@gmail.com> wrote: > From: Marek Olšák <marek.olsak@amd.com> > > It warns for all "{}" initializers. Well, I want us to use {}. > --- > configure.ac | 3 ++- > intel/intel_decode.c | 2 -- > 2 files changed, 2 insertions(+), 3 deletions(-) > > diff --git a/configure.ac b/configure.ac > index c8c4ace..057a846 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -174,7 +174,8 @@ MAYBE_WARN="-Wall -Wextra \ > -Wstrict-aliasing=2 -Winit-self \ > -Wdeclaration-after-statement -Wold-style-definition \ > -Wno-unused-parameter \ > --Wno-attributes -Wno-long-long -Winline -Wshadow" > +-Wno-attributes -Wno-long-long -Winline -Wshadow \ > +-Wno-missing-field-initializers" > > # invalidate cached value if MAYBE_WARN has changed > if test "x$libdrm_cv_warn_maybe" != "x$MAYBE_WARN"; then > diff --git a/intel/intel_decode.c b/intel/intel_decode.c > index e7aef74..287c342 100644 > --- a/intel/intel_decode.c > +++ b/intel/intel_decode.c > @@ -38,8 +38,6 @@ > #include "intel_chipset.h" > #include "intel_bufmgr.h" > > -/* The compiler throws ~90 warnings. Do not spam the build, until we fix them. */ > -#pragma GCC diagnostic ignored "-Wmissing-field-initializers" > > /* Struct for tracking drm_intel_decode state. */ > struct drm_intel_decode { > -- > 2.1.4 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
On 15 January 2016 at 17:24, Marek Olšák <maraeo@gmail.com> wrote: > On Fri, Jan 15, 2016 at 12:12 PM, Emil Velikov <emil.l.velikov@gmail.com> wrote: >> On 12 January 2016 at 23:14, Marek Olšák <maraeo@gmail.com> wrote: >>> From: Marek Olšák <marek.olsak@amd.com> >>> >>> It warns for all "{}" initializers. Well, I want us to use {}. >>> --- >>> configure.ac | 3 ++- >>> intel/intel_decode.c | 2 -- >> The whole of libdrm, minus the intel_decode can get away without using >> such constructs. And yes that includes radeon and amdgpu. >> >> NACK on this one - please be consistent with existing code base. > > Consistent with what? {} is the same as memset on each structure > member. The warning says that a structure member is initialized to > zero because of {}, which is why {} is used in the first place. It's > the same as using memset and getting a warning "memset initializes the > memory to zero". How useful is that? > There was a IRC discussion along the lines of "just use memset", but for the sake of me I cannot find it. > libdrm does have a lot of optional warnings enabled. Mesa does not, > and Mesa does not even have this one. This means libdrm is > inconsistent with Mesa and, BTW, it's also inconsistent with the kernel. > > It looks like somebody enabled optional warnings for libdrm in the > past. All I'm doing is aligning the behavior with Mesa/kernel, which > is what we would like to have and so would Intel apparently. > > Do you still think we are inconsistent? > If you look throughout libdrm you'll see - c99, {} (the one that's causing you problems ?) and {0} initializers. ... And zero warnings from Wmissing-field-initializers ? Don't know what your patch does, but if things flag that normally means "you're doing something new". If if bothers you that much - drop the warning. Just the next time please don't go for "I want", it feels a bit ... Thanks, Emil
On Mon, Jan 18, 2016 at 3:45 PM, Emil Velikov <emil.l.velikov@gmail.com> wrote: > On 15 January 2016 at 17:24, Marek Olšák <maraeo@gmail.com> wrote: >> On Fri, Jan 15, 2016 at 12:12 PM, Emil Velikov <emil.l.velikov@gmail.com> wrote: >>> On 12 January 2016 at 23:14, Marek Olšák <maraeo@gmail.com> wrote: >>>> From: Marek Olšák <marek.olsak@amd.com> >>>> >>>> It warns for all "{}" initializers. Well, I want us to use {}. >>>> --- >>>> configure.ac | 3 ++- >>>> intel/intel_decode.c | 2 -- >>> The whole of libdrm, minus the intel_decode can get away without using >>> such constructs. And yes that includes radeon and amdgpu. >>> >>> NACK on this one - please be consistent with existing code base. >> >> Consistent with what? {} is the same as memset on each structure >> member. The warning says that a structure member is initialized to >> zero because of {}, which is why {} is used in the first place. It's >> the same as using memset and getting a warning "memset initializes the >> memory to zero". How useful is that? >> > There was a IRC discussion along the lines of "just use memset", but > for the sake of me I cannot find it. > >> libdrm does have a lot of optional warnings enabled. Mesa does not, >> and Mesa does not even have this one. This means libdrm is >> inconsistent with Mesa and, BTW, it's also inconsistent with the kernel. >> >> It looks like somebody enabled optional warnings for libdrm in the >> past. All I'm doing is aligning the behavior with Mesa/kernel, which >> is what we would like to have and so would Intel apparently. >> >> Do you still think we are inconsistent? >> > If you look throughout libdrm you'll see - c99, {} (the one that's > causing you problems ?) and {0} initializers. ... And zero warnings > from Wmissing-field-initializers ? Don't know what your patch does, > but if things flag that normally means "you're doing something new". > > If if bothers you that much - drop the warning. Just the next time > please don't go for "I want", it feels a bit ... over the top? Sorry about that. The thing is libdrm enables too many warnings. It's annoying and they caused quite a lot of emotional discussion inside AMD. This is in configure.ac: MAYBE_WARN="-Wall -Wextra \ -Wsign-compare -Werror-implicit-function-declaration \ -Wpointer-arith -Wwrite-strings -Wstrict-prototypes \ -Wmissing-prototypes -Wmissing-declarations -Wnested-externs \ -Wpacked -Wswitch-enum -Wmissing-format-attribute \ -Wstrict-aliasing=2 -Winit-self \ -Wdeclaration-after-statement -Wold-style-definition \ -Wno-unused-parameter \ -Wno-attributes -Wno-long-long -Winline -Wshadow Marek
On Mon, 18 Jan 2016, Marek Olšák <maraeo@gmail.com> wrote: > On Mon, Jan 18, 2016 at 3:45 PM, Emil Velikov <emil.l.velikov@gmail.com> wrote: >> On 15 January 2016 at 17:24, Marek Olšák <maraeo@gmail.com> wrote: >>> On Fri, Jan 15, 2016 at 12:12 PM, Emil Velikov <emil.l.velikov@gmail.com> wrote: >>>> On 12 January 2016 at 23:14, Marek Olšák <maraeo@gmail.com> wrote: >>>>> From: Marek Olšák <marek.olsak@amd.com> >>>>> >>>>> It warns for all "{}" initializers. Well, I want us to use {}. >>>>> --- >>>>> configure.ac | 3 ++- >>>>> intel/intel_decode.c | 2 -- >>>> The whole of libdrm, minus the intel_decode can get away without using >>>> such constructs. And yes that includes radeon and amdgpu. >>>> >>>> NACK on this one - please be consistent with existing code base. >>> >>> Consistent with what? {} is the same as memset on each structure >>> member. The warning says that a structure member is initialized to >>> zero because of {}, which is why {} is used in the first place. It's >>> the same as using memset and getting a warning "memset initializes the >>> memory to zero". How useful is that? >>> >> There was a IRC discussion along the lines of "just use memset", but >> for the sake of me I cannot find it. >> >>> libdrm does have a lot of optional warnings enabled. Mesa does not, >>> and Mesa does not even have this one. This means libdrm is >>> inconsistent with Mesa and, BTW, it's also inconsistent with the kernel. >>> >>> It looks like somebody enabled optional warnings for libdrm in the >>> past. All I'm doing is aligning the behavior with Mesa/kernel, which >>> is what we would like to have and so would Intel apparently. >>> >>> Do you still think we are inconsistent? >>> >> If you look throughout libdrm you'll see - c99, {} (the one that's >> causing you problems ?) and {0} initializers. ... And zero warnings >> from Wmissing-field-initializers ? Don't know what your patch does, >> but if things flag that normally means "you're doing something new". >> >> If if bothers you that much - drop the warning. Just the next time >> please don't go for "I want", it feels a bit ... > > over the top? Sorry about that. > > The thing is libdrm enables too many warnings. It's annoying and they > caused quite a lot of emotional discussion inside AMD. This is in configure.ac: Please try upgrading your compiler. BR, Jani. > > MAYBE_WARN="-Wall -Wextra \ > -Wsign-compare -Werror-implicit-function-declaration \ > -Wpointer-arith -Wwrite-strings -Wstrict-prototypes \ > -Wmissing-prototypes -Wmissing-declarations -Wnested-externs \ > -Wpacked -Wswitch-enum -Wmissing-format-attribute \ > -Wstrict-aliasing=2 -Winit-self \ > -Wdeclaration-after-statement -Wold-style-definition \ > -Wno-unused-parameter \ > -Wno-attributes -Wno-long-long -Winline -Wshadow > > Marek > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
On 18 January 2016 at 17:43, Marek Olšák <maraeo@gmail.com> wrote: > On Mon, Jan 18, 2016 at 3:45 PM, Emil Velikov <emil.l.velikov@gmail.com> wrote: >> On 15 January 2016 at 17:24, Marek Olšák <maraeo@gmail.com> wrote: >>> On Fri, Jan 15, 2016 at 12:12 PM, Emil Velikov <emil.l.velikov@gmail.com> wrote: >>>> On 12 January 2016 at 23:14, Marek Olšák <maraeo@gmail.com> wrote: >>>>> From: Marek Olšák <marek.olsak@amd.com> >>>>> >>>>> It warns for all "{}" initializers. Well, I want us to use {}. >>>>> --- >>>>> configure.ac | 3 ++- >>>>> intel/intel_decode.c | 2 -- >>>> The whole of libdrm, minus the intel_decode can get away without using >>>> such constructs. And yes that includes radeon and amdgpu. >>>> >>>> NACK on this one - please be consistent with existing code base. >>> >>> Consistent with what? {} is the same as memset on each structure >>> member. The warning says that a structure member is initialized to >>> zero because of {}, which is why {} is used in the first place. It's >>> the same as using memset and getting a warning "memset initializes the >>> memory to zero". How useful is that? >>> >> There was a IRC discussion along the lines of "just use memset", but >> for the sake of me I cannot find it. >> >>> libdrm does have a lot of optional warnings enabled. Mesa does not, >>> and Mesa does not even have this one. This means libdrm is >>> inconsistent with Mesa and, BTW, it's also inconsistent with the kernel. >>> >>> It looks like somebody enabled optional warnings for libdrm in the >>> past. All I'm doing is aligning the behavior with Mesa/kernel, which >>> is what we would like to have and so would Intel apparently. >>> >>> Do you still think we are inconsistent? >>> >> If you look throughout libdrm you'll see - c99, {} (the one that's >> causing you problems ?) and {0} initializers. ... And zero warnings >> from Wmissing-field-initializers ? Don't know what your patch does, >> but if things flag that normally means "you're doing something new". >> >> If if bothers you that much - drop the warning. Just the next time >> please don't go for "I want", it feels a bit ... > > over the top? Sorry about that. > Precisely. Apology accepted :-) > The thing is libdrm enables too many warnings. It's annoying and they > caused quite a lot of emotional discussion inside AMD. This is in configure.ac: > > MAYBE_WARN="-Wall -Wextra \ > -Wsign-compare -Werror-implicit-function-declaration \ > -Wpointer-arith -Wwrite-strings -Wstrict-prototypes \ > -Wmissing-prototypes -Wmissing-declarations -Wnested-externs \ > -Wpacked -Wswitch-enum -Wmissing-format-attribute \ > -Wstrict-aliasing=2 -Winit-self \ > -Wdeclaration-after-statement -Wold-style-definition \ > -Wno-unused-parameter \ > -Wno-attributes -Wno-long-long -Winline -Wshadow > A few of those are already implicit with either Wall or Wextra. Both of which, imho, are a must have for any serious project. If you want we can nuke the -Wno-foo ones :-P But seriously - it makes me think that people are rushed to write the code and get it out. Or perhaps a too strong "no warnings" policy ? After all warnings are to hint that things can be improved/might be wrong. If it looks trivial, just ignore it :-) Cheers, Emil
On Mon, Jan 18, 2016 at 5:05 PM, Emil Velikov <emil.l.velikov@gmail.com> wrote: > On 18 January 2016 at 17:43, Marek Olšák <maraeo@gmail.com> wrote: >> On Mon, Jan 18, 2016 at 3:45 PM, Emil Velikov <emil.l.velikov@gmail.com> wrote: >>> On 15 January 2016 at 17:24, Marek Olšák <maraeo@gmail.com> wrote: >>>> On Fri, Jan 15, 2016 at 12:12 PM, Emil Velikov <emil.l.velikov@gmail.com> wrote: >>>>> On 12 January 2016 at 23:14, Marek Olšák <maraeo@gmail.com> wrote: >>>>>> From: Marek Olšák <marek.olsak@amd.com> >>>>>> >>>>>> It warns for all "{}" initializers. Well, I want us to use {}. >>>>>> --- >>>>>> configure.ac | 3 ++- >>>>>> intel/intel_decode.c | 2 -- >>>>> The whole of libdrm, minus the intel_decode can get away without using >>>>> such constructs. And yes that includes radeon and amdgpu. >>>>> >>>>> NACK on this one - please be consistent with existing code base. >>>> >>>> Consistent with what? {} is the same as memset on each structure >>>> member. The warning says that a structure member is initialized to >>>> zero because of {}, which is why {} is used in the first place. It's >>>> the same as using memset and getting a warning "memset initializes the >>>> memory to zero". How useful is that? >>>> >>> There was a IRC discussion along the lines of "just use memset", but >>> for the sake of me I cannot find it. >>> >>>> libdrm does have a lot of optional warnings enabled. Mesa does not, >>>> and Mesa does not even have this one. This means libdrm is >>>> inconsistent with Mesa and, BTW, it's also inconsistent with the kernel. >>>> >>>> It looks like somebody enabled optional warnings for libdrm in the >>>> past. All I'm doing is aligning the behavior with Mesa/kernel, which >>>> is what we would like to have and so would Intel apparently. >>>> >>>> Do you still think we are inconsistent? >>>> >>> If you look throughout libdrm you'll see - c99, {} (the one that's >>> causing you problems ?) and {0} initializers. ... And zero warnings >>> from Wmissing-field-initializers ? Don't know what your patch does, >>> but if things flag that normally means "you're doing something new". >>> >>> If if bothers you that much - drop the warning. Just the next time >>> please don't go for "I want", it feels a bit ... >> >> over the top? Sorry about that. >> > Precisely. Apology accepted :-) > >> The thing is libdrm enables too many warnings. It's annoying and they >> caused quite a lot of emotional discussion inside AMD. This is in configure.ac: >> >> MAYBE_WARN="-Wall -Wextra \ >> -Wsign-compare -Werror-implicit-function-declaration \ >> -Wpointer-arith -Wwrite-strings -Wstrict-prototypes \ >> -Wmissing-prototypes -Wmissing-declarations -Wnested-externs \ >> -Wpacked -Wswitch-enum -Wmissing-format-attribute \ >> -Wstrict-aliasing=2 -Winit-self \ >> -Wdeclaration-after-statement -Wold-style-definition \ >> -Wno-unused-parameter \ >> -Wno-attributes -Wno-long-long -Winline -Wshadow >> > A few of those are already implicit with either Wall or Wextra. Both > of which, imho, are a must have for any serious project. If you want > we can nuke the -Wno-foo ones :-P > > But seriously - it makes me think that people are rushed to write the > code and get it out. Or perhaps a too strong "no warnings" policy ? > After all warnings are to hint that things can be improved/might be > wrong. If it looks trivial, just ignore it :-) Try explaining that to people who have a compulsion to fix them or argue about them. :) Ignore? REALLY? IGNORE??? Marek
On 19.01.2016 01:05, Emil Velikov wrote: > > A few of those are already implicit with either Wall or Wextra. Both > of which, imho, are a must have for any serious project. I think -Wextra is generally too noisy for that, but I guess we're now deeply in arguing about taste territory. > But seriously - it makes me think that people are rushed to write the > code and get it out. Or perhaps a too strong "no warnings" policy ? > After all warnings are to hint that things can be improved/might be > wrong. If it looks trivial, just ignore it :-) One problem with that is that leaving trivial/irrelevant/incorrect warnings makes it easier to miss important warnings. That being said, I fully agree that one should resist the urge to just get rid of warnings in whatever way. (I tend to cringe whenever a commit log says something along the lines of "fix warning" — a change either fixes a problem which was pointed out by the warning, or it just silences the warning.)
On 18 January 2016 at 22:53, Marek Olšák <maraeo@gmail.com> wrote: > Try explaining that to people who have a compulsion to fix them or > argue about them. :) Ignore? REALLY? IGNORE??? > Now that we have a few people off your back can you please point out where this triggers warnings ? I've tried multiple times to get some but I'm falling short. Iirc this warning helped my catch an issue when cunit broke their ABI. Also I would kindly invite everyone else interested to speak publicly about their conserns - it is an FOSS project after all :-) Thanks Emil
On Thu, Jan 21, 2016 at 11:51 AM, Emil Velikov <emil.l.velikov@gmail.com> wrote: > On 18 January 2016 at 22:53, Marek Olšák <maraeo@gmail.com> wrote: >> Try explaining that to people who have a compulsion to fix them or >> argue about them. :) Ignore? REALLY? IGNORE??? >> > Now that we have a few people off your back can you please point out > where this triggers warnings ? This particular warning is trigged by {} or any { ... } which doesn't initialize all members. Marek
On 21 January 2016 at 12:08, Marek Olšák <maraeo@gmail.com> wrote: > On Thu, Jan 21, 2016 at 11:51 AM, Emil Velikov <emil.l.velikov@gmail.com> wrote: >> On 18 January 2016 at 22:53, Marek Olšák <maraeo@gmail.com> wrote: >>> Try explaining that to people who have a compulsion to fix them or >>> argue about them. :) Ignore? REALLY? IGNORE??? >>> >> Now that we have a few people off your back can you please point out >> where this triggers warnings ? > > This particular warning is trigged by {} As mentioned previously neither {} nor {0} trigger any warning here. Jani hinted that you might be using an old (buggy?) compiler which generates them. Which version of GCC are you using ? Do you mind showing the first few warnings ? > or any { ... } which doesn't > initialize all members. > Do we have any outside of intel_decode.c ? I'm failing to spot any. -Emil
On Thu, Jan 21, 2016 at 2:09 PM, Emil Velikov <emil.l.velikov@gmail.com> wrote: > On 21 January 2016 at 12:08, Marek Olšák <maraeo@gmail.com> wrote: >> On Thu, Jan 21, 2016 at 11:51 AM, Emil Velikov <emil.l.velikov@gmail.com> wrote: >>> On 18 January 2016 at 22:53, Marek Olšák <maraeo@gmail.com> wrote: >>>> Try explaining that to people who have a compulsion to fix them or >>>> argue about them. :) Ignore? REALLY? IGNORE??? >>>> >>> Now that we have a few people off your back can you please point out >>> where this triggers warnings ? >> >> This particular warning is trigged by {} > As mentioned previously neither {} nor {0} trigger any warning here. > Jani hinted that you might be using an old (buggy?) compiler which > generates them. > Which version of GCC are you using ? Do you mind showing the first few > warnings ? > >> or any { ... } which doesn't >> initialize all members. >> > Do we have any outside of intel_decode.c ? I'm failing to spot any. amdgpu_bo.c has 7 occurences of "= {}" and they all print the warning. There are more in libdrm. I have gcc 4.9.2. If I revert this patch, I get this nice log: xf86drmMode.c: In function ‘drmHandleEvent’: xf86drmMode.c:891:15: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing] e = (struct drm_event *) &buffer[i]; ^ abi16.c: In function ‘abi16_chan_nvc0’: abi16.c:66:9: warning: missing initializer for field ‘fb_ctxdma_handle’ of ‘struct drm_nouveau_channel_alloc’ [-Wmissing-field-initializers] struct drm_nouveau_channel_alloc req = {}; ^ In file included from private.h:8:0, from abi16.c:34: ../include/drm/nouveau_drm.h:31:11: note: ‘fb_ctxdma_handle’ declared here uint32_t fb_ctxdma_handle; ^ abi16.c: In function ‘abi16_chan_nve0’: abi16.c:87:9: warning: missing initializer for field ‘fb_ctxdma_handle’ of ‘struct drm_nouveau_channel_alloc’ [-Wmissing-field-initializers] struct drm_nouveau_channel_alloc req = {}; ^ In file included from private.h:8:0, from abi16.c:34: ../include/drm/nouveau_drm.h:31:11: note: ‘fb_ctxdma_handle’ declared here uint32_t fb_ctxdma_handle; ^ abi16.c: In function ‘abi16_bo_init’: abi16.c:316:9: warning: missing initializer for field ‘info’ of ‘struct drm_nouveau_gem_new’ [-Wmissing-field-initializers] struct drm_nouveau_gem_new req = {}; ^ In file included from private.h:8:0, from abi16.c:34: ../include/drm/nouveau_drm.h:118:30: note: ‘info’ declared here struct drm_nouveau_gem_info info; ^ nouveau.c: In function ‘nouveau_object_fini’: nouveau.c:222:2: warning: missing initializer for field ‘pad02’ of ‘struct nvif_ioctl_v0’ [-Wmissing-field-initializers] }; ^ In file included from nouveau.c:50:0: nvif/ioctl.h:22:7: note: ‘pad02’ declared here __u8 pad02[4]; ^ nouveau.c: In function ‘nouveau_device_new’: nouveau.c:371:9: warning: missing initializer for field ‘version’ of ‘struct nv_device_info_v0’ [-Wmissing-field-initializers] struct nv_device_info_v0 info = {}; ^ In file included from nouveau.c:49:0: nvif/cl0080.h:14:7: note: ‘version’ declared here __u8 version; ^ pushbuf.c: In function ‘nouveau_pushbuf_new’: pushbuf.c:544:9: warning: missing initializer for field ‘channel’ of ‘struct drm_nouveau_gem_pushbuf’ [-Wmissing-field-initializers] struct drm_nouveau_gem_pushbuf req = {}; ^ In file included from pushbuf.c:40:0: ../include/drm/nouveau_drm.h:162:11: note: ‘channel’ declared here uint32_t channel; ^ radeon_bo_gem.c: In function ‘bo_get_tiling’: radeon_bo_gem.c:255:12: warning: missing initializer for field ‘handle’ of ‘struct drm_radeon_gem_set_tiling’ [-Wmissing-field-initializers] struct drm_radeon_gem_set_tiling args = {}; ^ In file included from radeon_bo_gem.c:44:0: ../include/drm/radeon_drm.h:829:11: note: ‘handle’ declared here uint32_t handle; ^ radeon_cs_gem.c: In function ‘radeon_get_device_id’: radeon_cs_gem.c:531:12: warning: missing initializer for field ‘request’ of ‘struct drm_radeon_info’ [-Wmissing-field-initializers] struct drm_radeon_info info = {}; ^ In file included from radeon_cs.h:38:0, from radeon_cs_gem.c:41: ../include/drm/radeon_drm.h:1014:11: note: ‘request’ declared here uint32_t request; ^ radeon_surface.c: In function ‘radeon_get_value’: radeon_surface.c:121:12: warning: missing initializer for field ‘request’ of ‘struct drm_radeon_info’ [-Wmissing-field-initializers] struct drm_radeon_info info = {}; ^ In file included from radeon_surface.c:42:0: ../include/drm/radeon_drm.h:1014:11: note: ‘request’ declared here uint32_t request; ^ amdgpu_bo.c: In function ‘amdgpu_close_kms_handle’: amdgpu_bo.c:50:9: warning: missing initializer for field ‘handle’ of ‘struct drm_gem_close’ [-Wmissing-field-initializers] struct drm_gem_close args = {}; ^ In file included from ../xf86drm.h:40:0, from amdgpu_bo.c:41: ../include/drm/drm.h:590:8: note: ‘handle’ declared here __u32 handle; ^ amdgpu_bo.c: In function ‘amdgpu_bo_set_metadata’: amdgpu_bo.c:127:9: warning: missing initializer for field ‘handle’ of ‘struct drm_amdgpu_gem_metadata’ [-Wmissing-field-initializers] struct drm_amdgpu_gem_metadata args = {}; ^ In file included from amdgpu_bo.c:42:0: ../include/drm/amdgpu_drm.h:231:11: note: ‘handle’ declared here uint32_t handle; ^ amdgpu_bo.c: In function ‘amdgpu_bo_query_info’: amdgpu_bo.c:150:9: warning: missing initializer for field ‘handle’ of ‘struct drm_amdgpu_gem_metadata’ [-Wmissing-field-initializers] struct drm_amdgpu_gem_metadata metadata = {}; ^ In file included from amdgpu_bo.c:42:0: ../include/drm/amdgpu_drm.h:231:11: note: ‘handle’ declared here uint32_t handle; ^ amdgpu_bo.c:151:9: warning: missing initializer for field ‘bo_size’ of ‘struct drm_amdgpu_gem_create_in’ [-Wmissing-field-initializers] struct drm_amdgpu_gem_create_in bo_info = {}; ^ In file included from amdgpu_bo.c:42:0: ../include/drm/amdgpu_drm.h:81:11: note: ‘bo_size’ declared here uint64_t bo_size; ^ amdgpu_bo.c:152:9: warning: missing initializer for field ‘handle’ of ‘struct drm_amdgpu_gem_op’ [-Wmissing-field-initializers] struct drm_amdgpu_gem_op gem_op = {}; ^ In file included from amdgpu_bo.c:42:0: ../include/drm/amdgpu_drm.h:334:11: note: ‘handle’ declared here uint32_t handle; ^ amdgpu_bo.c: In function ‘amdgpu_bo_export_flink’: amdgpu_bo.c:240:10: warning: missing initializer for field ‘handle’ of ‘struct drm_gem_close’ [-Wmissing-field-initializers] struct drm_gem_close args = {}; ^ In file included from ../xf86drm.h:40:0, from amdgpu_bo.c:41: ../include/drm/drm.h:590:8: note: ‘handle’ declared here __u32 handle; ^ amdgpu_bo.c: In function ‘amdgpu_bo_import’: amdgpu_bo.c:287:9: warning: missing initializer for field ‘name’ of ‘struct drm_gem_open’ [-Wmissing-field-initializers] struct drm_gem_open open_arg = {}; ^ In file included from ../xf86drm.h:40:0, from amdgpu_bo.c:41: ../include/drm/drm.h:606:8: note: ‘name’ declared here __u32 name; ^ amdgpu_gpu_info.c: In function ‘amdgpu_query_heap_info’: amdgpu_gpu_info.c:240:9: warning: missing initializer for field ‘vram_size’ of ‘struct drm_amdgpu_info_vram_gtt’ [-Wmissing-field-initializers] struct drm_amdgpu_info_vram_gtt vram_gtt_info = {}; ^ In file included from amdgpu_gpu_info.c:33:0: ../include/drm/amdgpu_drm.h:586:11: note: ‘vram_size’ declared here uint64_t vram_size; ^ amdgpu_gpu_info.c: In function ‘amdgpu_query_gds_info’: amdgpu_gpu_info.c:290:9: warning: missing initializer for field ‘gds_gfx_partition_size’ of ‘struct drm_amdgpu_info_gds’ [-Wmissing-field-initializers] struct drm_amdgpu_info_gds gds_config = {}; ^ In file included from amdgpu_gpu_info.c:33:0: ../include/drm/amdgpu_drm.h:569:11: note: ‘gds_gfx_partition_size’ declared here uint32_t gds_gfx_partition_size; ^ amdgpu_device.c: In function ‘amdgpu_get_auth’: amdgpu_device.c:118:2: warning: missing initializer for field ‘idx’ of ‘drm_client_t’ [-Wmissing-field-initializers] drm_client_t client = {}; ^ In file included from ../xf86drm.h:40:0, from amdgpu_device.c:42: ../include/drm/drm.h:227:6: note: ‘idx’ declared here int idx; /**< Which client desired? */ ^ kms-universal-planes.c: In function ‘main’: kms-universal-planes.c:212:22: warning: declaration of ‘plane’ shadows a previous local [-Wshadow] struct kms_plane *plane = device->planes[i]; ^ kms-universal-planes.c:137:20: warning: shadowed declaration is here [-Wshadow] struct kms_plane *plane; ^ modetest.c: In function ‘get_resources’: modetest.c:559:3: warning: ignoring return value of ‘asprintf’, declared with attribute warn_unused_result [-Wunused-result] asprintf(&connector->name, "%s-%u", ^ basic_tests.c: In function ‘amdgpu_userptr_test’: basic_tests.c:1028:2: warning: ignoring return value of ‘posix_memalign’, declared with attribute warn_unused_result [-Wunused-result] posix_memalign(&ptr, sysconf(_SC_PAGE_SIZE), BUFFER_SIZE); ^ Marek
On 21 January 2016 at 16:58, Marek Olšák <maraeo@gmail.com> wrote: > On Thu, Jan 21, 2016 at 2:09 PM, Emil Velikov <emil.l.velikov@gmail.com> wrote: >> On 21 January 2016 at 12:08, Marek Olšák <maraeo@gmail.com> wrote: >>> On Thu, Jan 21, 2016 at 11:51 AM, Emil Velikov <emil.l.velikov@gmail.com> wrote: >>>> On 18 January 2016 at 22:53, Marek Olšák <maraeo@gmail.com> wrote: >>>>> Try explaining that to people who have a compulsion to fix them or >>>>> argue about them. :) Ignore? REALLY? IGNORE??? >>>>> >>>> Now that we have a few people off your back can you please point out >>>> where this triggers warnings ? >>> >>> This particular warning is trigged by {} >> As mentioned previously neither {} nor {0} trigger any warning here. >> Jani hinted that you might be using an old (buggy?) compiler which >> generates them. >> Which version of GCC are you using ? Do you mind showing the first few >> warnings ? >> >>> or any { ... } which doesn't >>> initialize all members. >>> >> Do we have any outside of intel_decode.c ? I'm failing to spot any. > > amdgpu_bo.c has 7 occurences of "= {}" and they all print the warning. With 200+ cases of memset and 40+ of "= *{ *0 *}". Any objections if I send a patch to transition to either one of these two ? > There are more in libdrm. I have gcc 4.9.2. If I revert this patch, I > get this nice log: > Interesting... I could swear I've tried the patch with gcc 4.9.x (currenly using 5.x). Hmm at the same time gcc does not seem to list any -Wunused-result warnings here. Thanks Emil
On Fri, Jan 22, 2016 at 6:13 PM, Emil Velikov <emil.l.velikov@gmail.com> wrote: > On 21 January 2016 at 16:58, Marek Olšák <maraeo@gmail.com> wrote: >> On Thu, Jan 21, 2016 at 2:09 PM, Emil Velikov <emil.l.velikov@gmail.com> wrote: >>> On 21 January 2016 at 12:08, Marek Olšák <maraeo@gmail.com> wrote: >>>> On Thu, Jan 21, 2016 at 11:51 AM, Emil Velikov <emil.l.velikov@gmail.com> wrote: >>>>> On 18 January 2016 at 22:53, Marek Olšák <maraeo@gmail.com> wrote: >>>>>> Try explaining that to people who have a compulsion to fix them or >>>>>> argue about them. :) Ignore? REALLY? IGNORE??? >>>>>> >>>>> Now that we have a few people off your back can you please point out >>>>> where this triggers warnings ? >>>> >>>> This particular warning is trigged by {} >>> As mentioned previously neither {} nor {0} trigger any warning here. >>> Jani hinted that you might be using an old (buggy?) compiler which >>> generates them. >>> Which version of GCC are you using ? Do you mind showing the first few >>> warnings ? >>> >>>> or any { ... } which doesn't >>>> initialize all members. >>>> >>> Do we have any outside of intel_decode.c ? I'm failing to spot any. >> >> amdgpu_bo.c has 7 occurences of "= {}" and they all print the warning. > With 200+ cases of memset and 40+ of "= *{ *0 *}". Any objections if I > send a patch to transition to either one of these two ? That's up to you, but please note that I don't plan to stop using "= {}", because it's the most convenient way to clear memory in a lot of cases and takes only 4 bytes of text. Marek
On Fri, Jan 22, 2016 at 12:18 PM, Marek Olšák <maraeo@gmail.com> wrote: > On Fri, Jan 22, 2016 at 6:13 PM, Emil Velikov <emil.l.velikov@gmail.com> wrote: >> On 21 January 2016 at 16:58, Marek Olšák <maraeo@gmail.com> wrote: >>> On Thu, Jan 21, 2016 at 2:09 PM, Emil Velikov <emil.l.velikov@gmail.com> wrote: >>>> On 21 January 2016 at 12:08, Marek Olšák <maraeo@gmail.com> wrote: >>>>> On Thu, Jan 21, 2016 at 11:51 AM, Emil Velikov <emil.l.velikov@gmail.com> wrote: >>>>>> On 18 January 2016 at 22:53, Marek Olšák <maraeo@gmail.com> wrote: >>>>>>> Try explaining that to people who have a compulsion to fix them or >>>>>>> argue about them. :) Ignore? REALLY? IGNORE??? >>>>>>> >>>>>> Now that we have a few people off your back can you please point out >>>>>> where this triggers warnings ? >>>>> >>>>> This particular warning is trigged by {} >>>> As mentioned previously neither {} nor {0} trigger any warning here. >>>> Jani hinted that you might be using an old (buggy?) compiler which >>>> generates them. >>>> Which version of GCC are you using ? Do you mind showing the first few >>>> warnings ? >>>> >>>>> or any { ... } which doesn't >>>>> initialize all members. >>>>> >>>> Do we have any outside of intel_decode.c ? I'm failing to spot any. >>> >>> amdgpu_bo.c has 7 occurences of "= {}" and they all print the warning. >> With 200+ cases of memset and 40+ of "= *{ *0 *}". Any objections if I >> send a patch to transition to either one of these two ? > > That's up to you, but please note that I don't plan to stop using "= {}", > because it's the most convenient way to clear memory in a lot of > cases and takes only 4 bytes of text. I like {} too and think we should encourage that. I'd rather transition the { 0 } stuff over to {}. -ilia
On 22 January 2016 at 17:29, Ilia Mirkin <imirkin@alum.mit.edu> wrote: > On Fri, Jan 22, 2016 at 12:18 PM, Marek Olšák <maraeo@gmail.com> wrote: >> On Fri, Jan 22, 2016 at 6:13 PM, Emil Velikov <emil.l.velikov@gmail.com> wrote: >>> On 21 January 2016 at 16:58, Marek Olšák <maraeo@gmail.com> wrote: >>>> On Thu, Jan 21, 2016 at 2:09 PM, Emil Velikov <emil.l.velikov@gmail.com> wrote: >>>>> On 21 January 2016 at 12:08, Marek Olšák <maraeo@gmail.com> wrote: >>>>>> On Thu, Jan 21, 2016 at 11:51 AM, Emil Velikov <emil.l.velikov@gmail.com> wrote: >>>>>>> On 18 January 2016 at 22:53, Marek Olšák <maraeo@gmail.com> wrote: >>>>>>>> Try explaining that to people who have a compulsion to fix them or >>>>>>>> argue about them. :) Ignore? REALLY? IGNORE??? >>>>>>>> >>>>>>> Now that we have a few people off your back can you please point out >>>>>>> where this triggers warnings ? >>>>>> >>>>>> This particular warning is trigged by {} >>>>> As mentioned previously neither {} nor {0} trigger any warning here. >>>>> Jani hinted that you might be using an old (buggy?) compiler which >>>>> generates them. >>>>> Which version of GCC are you using ? Do you mind showing the first few >>>>> warnings ? >>>>> >>>>>> or any { ... } which doesn't >>>>>> initialize all members. >>>>>> >>>>> Do we have any outside of intel_decode.c ? I'm failing to spot any. >>>> >>>> amdgpu_bo.c has 7 occurences of "= {}" and they all print the warning. >>> With 200+ cases of memset and 40+ of "= *{ *0 *}". Any objections if I >>> send a patch to transition to either one of these two ? >> >> That's up to you, but please note that I don't plan to stop using "= {}", >> because it's the most convenient way to clear memory in a lot of >> cases and takes only 4 bytes of text. > > I like {} too and think we should encourage that. I'd rather > transition the { 0 } stuff over to {}. > So people feel against seeing/writing single extra character 0, despite that the warning has helped catch actual bug ? And now are willing to transitions 40+ cases as opposed to ~15... that feels strange to say the least. -Emil
On Fri, Jan 22, 2016 at 05:40:54PM +0000, Emil Velikov wrote: > On 22 January 2016 at 17:29, Ilia Mirkin <imirkin@alum.mit.edu> wrote: > > On Fri, Jan 22, 2016 at 12:18 PM, Marek Olšák <maraeo@gmail.com> wrote: > >> On Fri, Jan 22, 2016 at 6:13 PM, Emil Velikov <emil.l.velikov@gmail.com> wrote: > >>> On 21 January 2016 at 16:58, Marek Olšák <maraeo@gmail.com> wrote: > >>>> On Thu, Jan 21, 2016 at 2:09 PM, Emil Velikov <emil.l.velikov@gmail.com> wrote: > >>>>> On 21 January 2016 at 12:08, Marek Olšák <maraeo@gmail.com> wrote: > >>>>>> On Thu, Jan 21, 2016 at 11:51 AM, Emil Velikov <emil.l.velikov@gmail.com> wrote: > >>>>>>> On 18 January 2016 at 22:53, Marek Olšák <maraeo@gmail.com> wrote: > >>>>>>>> Try explaining that to people who have a compulsion to fix them or > >>>>>>>> argue about them. :) Ignore? REALLY? IGNORE??? > >>>>>>>> > >>>>>>> Now that we have a few people off your back can you please point out > >>>>>>> where this triggers warnings ? > >>>>>> > >>>>>> This particular warning is trigged by {} > >>>>> As mentioned previously neither {} nor {0} trigger any warning here. > >>>>> Jani hinted that you might be using an old (buggy?) compiler which > >>>>> generates them. > >>>>> Which version of GCC are you using ? Do you mind showing the first few > >>>>> warnings ? > >>>>> > >>>>>> or any { ... } which doesn't > >>>>>> initialize all members. > >>>>>> > >>>>> Do we have any outside of intel_decode.c ? I'm failing to spot any. > >>>> > >>>> amdgpu_bo.c has 7 occurences of "= {}" and they all print the warning. > >>> With 200+ cases of memset and 40+ of "= *{ *0 *}". Any objections if I > >>> send a patch to transition to either one of these two ? > >> > >> That's up to you, but please note that I don't plan to stop using "= {}", > >> because it's the most convenient way to clear memory in a lot of > >> cases and takes only 4 bytes of text. > > > > I like {} too and think we should encourage that. I'd rather > > transition the { 0 } stuff over to {}. > > > So people feel against seeing/writing single extra character 0, > despite that the warning has helped catch actual bug ? > And now are willing to transitions 40+ cases as opposed to ~15... that > feels strange to say the least. Does the '= { 0 }' thing even work if the first member happens to be something other than an integer?
On Fri, Jan 22, 2016 at 12:47 PM, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > On Fri, Jan 22, 2016 at 05:40:54PM +0000, Emil Velikov wrote: >> On 22 January 2016 at 17:29, Ilia Mirkin <imirkin@alum.mit.edu> wrote: >> > On Fri, Jan 22, 2016 at 12:18 PM, Marek Olšák <maraeo@gmail.com> wrote: >> >> On Fri, Jan 22, 2016 at 6:13 PM, Emil Velikov <emil.l.velikov@gmail.com> wrote: >> >>> On 21 January 2016 at 16:58, Marek Olšák <maraeo@gmail.com> wrote: >> >>>> On Thu, Jan 21, 2016 at 2:09 PM, Emil Velikov <emil.l.velikov@gmail.com> wrote: >> >>>>> On 21 January 2016 at 12:08, Marek Olšák <maraeo@gmail.com> wrote: >> >>>>>> On Thu, Jan 21, 2016 at 11:51 AM, Emil Velikov <emil.l.velikov@gmail.com> wrote: >> >>>>>>> On 18 January 2016 at 22:53, Marek Olšák <maraeo@gmail.com> wrote: >> >>>>>>>> Try explaining that to people who have a compulsion to fix them or >> >>>>>>>> argue about them. :) Ignore? REALLY? IGNORE??? >> >>>>>>>> >> >>>>>>> Now that we have a few people off your back can you please point out >> >>>>>>> where this triggers warnings ? >> >>>>>> >> >>>>>> This particular warning is trigged by {} >> >>>>> As mentioned previously neither {} nor {0} trigger any warning here. >> >>>>> Jani hinted that you might be using an old (buggy?) compiler which >> >>>>> generates them. >> >>>>> Which version of GCC are you using ? Do you mind showing the first few >> >>>>> warnings ? >> >>>>> >> >>>>>> or any { ... } which doesn't >> >>>>>> initialize all members. >> >>>>>> >> >>>>> Do we have any outside of intel_decode.c ? I'm failing to spot any. >> >>>> >> >>>> amdgpu_bo.c has 7 occurences of "= {}" and they all print the warning. >> >>> With 200+ cases of memset and 40+ of "= *{ *0 *}". Any objections if I >> >>> send a patch to transition to either one of these two ? >> >> >> >> That's up to you, but please note that I don't plan to stop using "= {}", >> >> because it's the most convenient way to clear memory in a lot of >> >> cases and takes only 4 bytes of text. >> > >> > I like {} too and think we should encourage that. I'd rather >> > transition the { 0 } stuff over to {}. >> > >> So people feel against seeing/writing single extra character 0, >> despite that the warning has helped catch actual bug ? >> And now are willing to transitions 40+ cases as opposed to ~15... that >> feels strange to say the least. > > Does the '= { 0 }' thing even work if the first member happens to be > something other than an integer? No. That's why I like {}. Otherwise you end up doing {{{{{{{{{{0}}}}}}}}}. -ilia
On 22 January 2016 at 17:47, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > On Fri, Jan 22, 2016 at 05:40:54PM +0000, Emil Velikov wrote: >> On 22 January 2016 at 17:29, Ilia Mirkin <imirkin@alum.mit.edu> wrote: >> > On Fri, Jan 22, 2016 at 12:18 PM, Marek Olšák <maraeo@gmail.com> wrote: >> >> On Fri, Jan 22, 2016 at 6:13 PM, Emil Velikov <emil.l.velikov@gmail.com> wrote: >> >>> On 21 January 2016 at 16:58, Marek Olšák <maraeo@gmail.com> wrote: >> >>>> On Thu, Jan 21, 2016 at 2:09 PM, Emil Velikov <emil.l.velikov@gmail.com> wrote: >> >>>>> On 21 January 2016 at 12:08, Marek Olšák <maraeo@gmail.com> wrote: >> >>>>>> On Thu, Jan 21, 2016 at 11:51 AM, Emil Velikov <emil.l.velikov@gmail.com> wrote: >> >>>>>>> On 18 January 2016 at 22:53, Marek Olšák <maraeo@gmail.com> wrote: >> >>>>>>>> Try explaining that to people who have a compulsion to fix them or >> >>>>>>>> argue about them. :) Ignore? REALLY? IGNORE??? >> >>>>>>>> >> >>>>>>> Now that we have a few people off your back can you please point out >> >>>>>>> where this triggers warnings ? >> >>>>>> >> >>>>>> This particular warning is trigged by {} >> >>>>> As mentioned previously neither {} nor {0} trigger any warning here. >> >>>>> Jani hinted that you might be using an old (buggy?) compiler which >> >>>>> generates them. >> >>>>> Which version of GCC are you using ? Do you mind showing the first few >> >>>>> warnings ? >> >>>>> >> >>>>>> or any { ... } which doesn't >> >>>>>> initialize all members. >> >>>>>> >> >>>>> Do we have any outside of intel_decode.c ? I'm failing to spot any. >> >>>> >> >>>> amdgpu_bo.c has 7 occurences of "= {}" and they all print the warning. >> >>> With 200+ cases of memset and 40+ of "= *{ *0 *}". Any objections if I >> >>> send a patch to transition to either one of these two ? >> >> >> >> That's up to you, but please note that I don't plan to stop using "= {}", >> >> because it's the most convenient way to clear memory in a lot of >> >> cases and takes only 4 bytes of text. >> > >> > I like {} too and think we should encourage that. I'd rather >> > transition the { 0 } stuff over to {}. >> > >> So people feel against seeing/writing single extra character 0, >> despite that the warning has helped catch actual bug ? >> And now are willing to transitions 40+ cases as opposed to ~15... that >> feels strange to say the least. > > Does the '= { 0 }' thing even work if the first member happens to be > something other than an integer? > It does here with GCC 5.2.0 :-) Cannot comment about other compilers. -Emil
On 22 January 2016 at 17:50, Emil Velikov <emil.l.velikov@gmail.com> wrote: > On 22 January 2016 at 17:47, Ville Syrjälä > <ville.syrjala@linux.intel.com> wrote: >> On Fri, Jan 22, 2016 at 05:40:54PM +0000, Emil Velikov wrote: >>> On 22 January 2016 at 17:29, Ilia Mirkin <imirkin@alum.mit.edu> wrote: >>> > On Fri, Jan 22, 2016 at 12:18 PM, Marek Olšák <maraeo@gmail.com> wrote: >>> >> On Fri, Jan 22, 2016 at 6:13 PM, Emil Velikov <emil.l.velikov@gmail.com> wrote: >>> >>> On 21 January 2016 at 16:58, Marek Olšák <maraeo@gmail.com> wrote: >>> >>>> On Thu, Jan 21, 2016 at 2:09 PM, Emil Velikov <emil.l.velikov@gmail.com> wrote: >>> >>>>> On 21 January 2016 at 12:08, Marek Olšák <maraeo@gmail.com> wrote: >>> >>>>>> On Thu, Jan 21, 2016 at 11:51 AM, Emil Velikov <emil.l.velikov@gmail.com> wrote: >>> >>>>>>> On 18 January 2016 at 22:53, Marek Olšák <maraeo@gmail.com> wrote: >>> >>>>>>>> Try explaining that to people who have a compulsion to fix them or >>> >>>>>>>> argue about them. :) Ignore? REALLY? IGNORE??? >>> >>>>>>>> >>> >>>>>>> Now that we have a few people off your back can you please point out >>> >>>>>>> where this triggers warnings ? >>> >>>>>> >>> >>>>>> This particular warning is trigged by {} >>> >>>>> As mentioned previously neither {} nor {0} trigger any warning here. >>> >>>>> Jani hinted that you might be using an old (buggy?) compiler which >>> >>>>> generates them. >>> >>>>> Which version of GCC are you using ? Do you mind showing the first few >>> >>>>> warnings ? >>> >>>>> >>> >>>>>> or any { ... } which doesn't >>> >>>>>> initialize all members. >>> >>>>>> >>> >>>>> Do we have any outside of intel_decode.c ? I'm failing to spot any. >>> >>>> >>> >>>> amdgpu_bo.c has 7 occurences of "= {}" and they all print the warning. >>> >>> With 200+ cases of memset and 40+ of "= *{ *0 *}". Any objections if I >>> >>> send a patch to transition to either one of these two ? >>> >> >>> >> That's up to you, but please note that I don't plan to stop using "= {}", >>> >> because it's the most convenient way to clear memory in a lot of >>> >> cases and takes only 4 bytes of text. >>> > >>> > I like {} too and think we should encourage that. I'd rather >>> > transition the { 0 } stuff over to {}. >>> > >>> So people feel against seeing/writing single extra character 0, >>> despite that the warning has helped catch actual bug ? >>> And now are willing to transitions 40+ cases as opposed to ~15... that >>> feels strange to say the least. >> >> Does the '= { 0 }' thing even work if the first member happens to be >> something other than an integer? >> > It does here with GCC 5.2.0 :-) Cannot comment about other compilers. > Also let's not forget about a.c:17:20: warning: ISO C forbids empty initializer braces [-Wpedantic] struct foo f = {}; ^ -Emil
On Fri, Jan 22, 2016 at 05:59:30PM +0000, Emil Velikov wrote: > On 22 January 2016 at 17:50, Emil Velikov <emil.l.velikov@gmail.com> wrote: > > On 22 January 2016 at 17:47, Ville Syrjälä > > <ville.syrjala@linux.intel.com> wrote: > >> On Fri, Jan 22, 2016 at 05:40:54PM +0000, Emil Velikov wrote: > >>> On 22 January 2016 at 17:29, Ilia Mirkin <imirkin@alum.mit.edu> wrote: > >>> > On Fri, Jan 22, 2016 at 12:18 PM, Marek Olšák <maraeo@gmail.com> wrote: > >>> >> On Fri, Jan 22, 2016 at 6:13 PM, Emil Velikov <emil.l.velikov@gmail.com> wrote: > >>> >>> On 21 January 2016 at 16:58, Marek Olšák <maraeo@gmail.com> wrote: > >>> >>>> On Thu, Jan 21, 2016 at 2:09 PM, Emil Velikov <emil.l.velikov@gmail.com> wrote: > >>> >>>>> On 21 January 2016 at 12:08, Marek Olšák <maraeo@gmail.com> wrote: > >>> >>>>>> On Thu, Jan 21, 2016 at 11:51 AM, Emil Velikov <emil.l.velikov@gmail.com> wrote: > >>> >>>>>>> On 18 January 2016 at 22:53, Marek Olšák <maraeo@gmail.com> wrote: > >>> >>>>>>>> Try explaining that to people who have a compulsion to fix them or > >>> >>>>>>>> argue about them. :) Ignore? REALLY? IGNORE??? > >>> >>>>>>>> > >>> >>>>>>> Now that we have a few people off your back can you please point out > >>> >>>>>>> where this triggers warnings ? > >>> >>>>>> > >>> >>>>>> This particular warning is trigged by {} > >>> >>>>> As mentioned previously neither {} nor {0} trigger any warning here. > >>> >>>>> Jani hinted that you might be using an old (buggy?) compiler which > >>> >>>>> generates them. > >>> >>>>> Which version of GCC are you using ? Do you mind showing the first few > >>> >>>>> warnings ? > >>> >>>>> > >>> >>>>>> or any { ... } which doesn't > >>> >>>>>> initialize all members. > >>> >>>>>> > >>> >>>>> Do we have any outside of intel_decode.c ? I'm failing to spot any. > >>> >>>> > >>> >>>> amdgpu_bo.c has 7 occurences of "= {}" and they all print the warning. > >>> >>> With 200+ cases of memset and 40+ of "= *{ *0 *}". Any objections if I > >>> >>> send a patch to transition to either one of these two ? > >>> >> > >>> >> That's up to you, but please note that I don't plan to stop using "= {}", > >>> >> because it's the most convenient way to clear memory in a lot of > >>> >> cases and takes only 4 bytes of text. > >>> > > >>> > I like {} too and think we should encourage that. I'd rather > >>> > transition the { 0 } stuff over to {}. > >>> > > >>> So people feel against seeing/writing single extra character 0, > >>> despite that the warning has helped catch actual bug ? > >>> And now are willing to transitions 40+ cases as opposed to ~15... that > >>> feels strange to say the least. > >> > >> Does the '= { 0 }' thing even work if the first member happens to be > >> something other than an integer? > >> > > It does here with GCC 5.2.0 :-) Cannot comment about other compilers. > > > Also let's not forget about > > a.c:17:20: warning: ISO C forbids empty initializer braces [-Wpedantic] > struct foo f = {}; > ^ I long ago decided that -pedantic is stupid, hence I don't use it. My gcc (4.9.3 something) seems to allow the {0} but with a struct within a struct it angers -Wmissing-braces, although my reading of the spec suggests that it's pretty well defined how this sort of thing should behave. I was expecting some kind of 'implicit pointer from integer' warning when the thing it would initialize is a pointer, but didn't get one. Not sure why. And {} of course makes -Wmissing-field-initializers upset. I can't see anything in the spec to relly forbid this form, except that the syntax maybe doesn't allow for an empty initializer-list. About the only "useful" thing I learned from the spec is that 0 is an octal constant :) Makes some sense but it never occured to me before. So I guess all I can say is that gcc is stupid, and it should just stfu and let both '= {}' and '= {0}' through without whining about it.
On Fri, 2016-01-22 at 12:48 -0500, Ilia Mirkin wrote: > On Fri, Jan 22, 2016 at 12:47 PM, Ville Syrjälä > <ville.syrjala@linux.intel.com> wrote: > > On Fri, Jan 22, 2016 at 05:40:54PM +0000, Emil Velikov wrote: > > > On 22 January 2016 at 17:29, Ilia Mirkin <imirkin@alum.mit.edu> > > > wrote: > > > > On Fri, Jan 22, 2016 at 12:18 PM, Marek Olšák <maraeo@gmail.com > > > > > wrote: > > > > > On Fri, Jan 22, 2016 at 6:13 PM, Emil Velikov <emil.l.velikov > > > > > @gmail.com> wrote: > > > > > > On 21 January 2016 at 16:58, Marek Olšák <maraeo@gmail.com> > > > > > > wrote: > > > > > > > On Thu, Jan 21, 2016 at 2:09 PM, Emil Velikov <emil.l.vel > > > > > > > ikov@gmail.com> wrote: > > > > > > > > On 21 January 2016 at 12:08, Marek Olšák <maraeo@gmail. > > > > > > > > com> wrote: > > > > > > > > > On Thu, Jan 21, 2016 at 11:51 AM, Emil Velikov <emil. > > > > > > > > > l.velikov@gmail.com> wrote: > > > > > > > > > > On 18 January 2016 at 22:53, Marek Olšák <maraeo@gm > > > > > > > > > > ail.com> wrote: > > > > > > > > > > > Try explaining that to people who have a > > > > > > > > > > > compulsion to fix them or > > > > > > > > > > > argue about them. :) Ignore? REALLY? IGNORE??? > > > > > > > > > > > > > > > > > > > > > Now that we have a few people off your back can you > > > > > > > > > > please point out > > > > > > > > > > where this triggers warnings ? > > > > > > > > > > > > > > > > > > This particular warning is trigged by {} > > > > > > > > As mentioned previously neither {} nor {0} trigger any > > > > > > > > warning here. > > > > > > > > Jani hinted that you might be using an old (buggy?) > > > > > > > > compiler which > > > > > > > > generates them. > > > > > > > > Which version of GCC are you using ? Do you mind > > > > > > > > showing the first few > > > > > > > > warnings ? > > > > > > > > > > > > > > > > > or any { ... } which doesn't > > > > > > > > > initialize all members. > > > > > > > > > > > > > > > > > Do we have any outside of intel_decode.c ? I'm failing > > > > > > > > to spot any. > > > > > > > > > > > > > > amdgpu_bo.c has 7 occurences of "= {}" and they all print > > > > > > > the warning. > > > > > > With 200+ cases of memset and 40+ of "= *{ *0 *}". Any > > > > > > objections if I > > > > > > send a patch to transition to either one of these two ? > > > > > > > > > > That's up to you, but please note that I don't plan to stop > > > > > using "= {}", > > > > > because it's the most convenient way to clear memory in a lot > > > > > of > > > > > cases and takes only 4 bytes of text. > > > > > > > > I like {} too and think we should encourage that. I'd rather > > > > transition the { 0 } stuff over to {}. > > > > > > > So people feel against seeing/writing single extra character 0, > > > despite that the warning has helped catch actual bug ? > > > And now are willing to transitions 40+ cases as opposed to ~15... > > > that > > > feels strange to say the least. > > > > Does the '= { 0 }' thing even work if the first member happens to > > be > > something other than an integer? > > No. That's why I like {}. Otherwise you end up doing > {{{{{{{{{{0}}}}}}}}}. ISO C99 According to 6.7.8 20 all braces but the outermost ones are optional. {}, on the other hand, is not allowed by syntax rules. Jan > > -ilia > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
On 22 January 2016 at 18:49, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > On Fri, Jan 22, 2016 at 05:59:30PM +0000, Emil Velikov wrote: >> Also let's not forget about >> >> a.c:17:20: warning: ISO C forbids empty initializer braces [-Wpedantic] >> struct foo f = {}; >> ^ > > I long ago decided that -pedantic is stupid, hence I don't use it. > Only tried pedantic as I couldn't find any references to "= {}" in the C spec. I'm not even remotely suggesting that we use it. > My gcc (4.9.3 something) seems to allow the {0} but with a struct within > a struct it angers -Wmissing-braces, although my reading of the spec The -Wmissing-braces fix might get backported for 4.8 and 4.9 [1] > suggests that it's pretty well defined how this sort of thing should > behave. I was expecting some kind of 'implicit pointer from integer' > warning when the thing it would initialize is a pointer, but didn't get > one. Not sure why. > > And {} of course makes -Wmissing-field-initializers upset. I can't see > anything in the spec to relly forbid this form, except that the syntax > maybe doesn't allow for an empty initializer-list. > > About the only "useful" thing I learned from the spec is that 0 is an > octal constant :) Makes some sense but it never occured to me before. > > So I guess all I can say is that gcc is stupid, and it should just stfu > and let both '= {}' and '= {0}' through without whining about it. > Luckily with the 5 series things are shaping up :-) Thanks for digging it up. -Emil [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53119
On Fri, Jan 22, 2016 at 07:21:31PM +0000, Emil Velikov wrote: > On 22 January 2016 at 18:49, Ville Syrjälä > <ville.syrjala@linux.intel.com> wrote: > > On Fri, Jan 22, 2016 at 05:59:30PM +0000, Emil Velikov wrote: > > >> Also let's not forget about > >> > >> a.c:17:20: warning: ISO C forbids empty initializer braces [-Wpedantic] > >> struct foo f = {}; > >> ^ > > > > I long ago decided that -pedantic is stupid, hence I don't use it. > > > Only tried pedantic as I couldn't find any references to "= {}" in the > C spec. I'm not even remotely suggesting that we use it. > > > My gcc (4.9.3 something) seems to allow the {0} but with a struct within > > a struct it angers -Wmissing-braces, although my reading of the spec > The -Wmissing-braces fix might get backported for 4.8 and 4.9 [1] > > > suggests that it's pretty well defined how this sort of thing should > > behave. I was expecting some kind of 'implicit pointer from integer' > > warning when the thing it would initialize is a pointer, but didn't get > > one. Not sure why. > > > > And {} of course makes -Wmissing-field-initializers upset. I can't see > > anything in the spec to relly forbid this form, except that the syntax > > maybe doesn't allow for an empty initializer-list. > > > > About the only "useful" thing I learned from the spec is that 0 is an > > octal constant :) Makes some sense but it never occured to me before. > > > > So I guess all I can say is that gcc is stupid, and it should just stfu > > and let both '= {}' and '= {0}' through without whining about it. > > > Luckily with the 5 series things are shaping up :-) > > Thanks for digging it up. > -Emil > > [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53119 Hmm. So yet another C vs. C++ difference. I guess everyone has given up on the supposed ability of C++ to include C code.
diff --git a/configure.ac b/configure.ac index c8c4ace..057a846 100644 --- a/configure.ac +++ b/configure.ac @@ -174,7 +174,8 @@ MAYBE_WARN="-Wall -Wextra \ -Wstrict-aliasing=2 -Winit-self \ -Wdeclaration-after-statement -Wold-style-definition \ -Wno-unused-parameter \ --Wno-attributes -Wno-long-long -Winline -Wshadow" +-Wno-attributes -Wno-long-long -Winline -Wshadow \ +-Wno-missing-field-initializers" # invalidate cached value if MAYBE_WARN has changed if test "x$libdrm_cv_warn_maybe" != "x$MAYBE_WARN"; then diff --git a/intel/intel_decode.c b/intel/intel_decode.c index e7aef74..287c342 100644 --- a/intel/intel_decode.c +++ b/intel/intel_decode.c @@ -38,8 +38,6 @@ #include "intel_chipset.h" #include "intel_bufmgr.h" -/* The compiler throws ~90 warnings. Do not spam the build, until we fix them. */ -#pragma GCC diagnostic ignored "-Wmissing-field-initializers" /* Struct for tracking drm_intel_decode state. */ struct drm_intel_decode {
From: Marek Olšák <marek.olsak@amd.com> It warns for all "{}" initializers. Well, I want us to use {}. --- configure.ac | 3 ++- intel/intel_decode.c | 2 -- 2 files changed, 2 insertions(+), 3 deletions(-)