All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] configure.ac: disable annoying warning -Wmissing-field-initializers
@ 2016-01-12 21:14 Marek Olšák
  2016-01-15 11:12 ` Emil Velikov
  2016-01-16 19:49 ` Ilia Mirkin
  0 siblings, 2 replies; 27+ messages in thread
From: Marek Olšák @ 2016-01-12 21:14 UTC (permalink / raw)
  To: dri-devel

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

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* Re: [PATCH] configure.ac: disable annoying warning -Wmissing-field-initializers
  2016-01-12 21:14 [PATCH] configure.ac: disable annoying warning -Wmissing-field-initializers Marek Olšák
@ 2016-01-15 11:12 ` Emil Velikov
  2016-01-15 15:24   ` Marek Olšák
  2016-01-16 19:49 ` Ilia Mirkin
  1 sibling, 1 reply; 27+ messages in thread
From: Emil Velikov @ 2016-01-15 11:12 UTC (permalink / raw)
  To: Marek Olšák; +Cc: ML dri-devel

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] configure.ac: disable annoying warning -Wmissing-field-initializers
  2016-01-15 11:12 ` Emil Velikov
@ 2016-01-15 15:24   ` Marek Olšák
  2016-01-16 19:46     ` David Herrmann
  2016-01-18 14:45     ` Emil Velikov
  0 siblings, 2 replies; 27+ messages in thread
From: Marek Olšák @ 2016-01-15 15:24 UTC (permalink / raw)
  To: Emil Velikov; +Cc: ML dri-devel

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] configure.ac: disable annoying warning -Wmissing-field-initializers
  2016-01-15 15:24   ` Marek Olšák
@ 2016-01-16 19:46     ` David Herrmann
  2016-01-18 14:45     ` Emil Velikov
  1 sibling, 0 replies; 27+ messages in thread
From: David Herrmann @ 2016-01-16 19:46 UTC (permalink / raw)
  To: Marek Olšák; +Cc: Emil Velikov, ML dri-devel

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] configure.ac: disable annoying warning -Wmissing-field-initializers
  2016-01-12 21:14 [PATCH] configure.ac: disable annoying warning -Wmissing-field-initializers Marek Olšák
  2016-01-15 11:12 ` Emil Velikov
@ 2016-01-16 19:49 ` Ilia Mirkin
  1 sibling, 0 replies; 27+ messages in thread
From: Ilia Mirkin @ 2016-01-16 19:49 UTC (permalink / raw)
  To: Marek Olšák; +Cc: dri-devel

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] configure.ac: disable annoying warning -Wmissing-field-initializers
  2016-01-15 15:24   ` Marek Olšák
  2016-01-16 19:46     ` David Herrmann
@ 2016-01-18 14:45     ` Emil Velikov
  2016-01-18 15:43       ` Marek Olšák
  1 sibling, 1 reply; 27+ messages in thread
From: Emil Velikov @ 2016-01-18 14:45 UTC (permalink / raw)
  To: Marek Olšák; +Cc: ML 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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] configure.ac: disable annoying warning -Wmissing-field-initializers
  2016-01-18 14:45     ` Emil Velikov
@ 2016-01-18 15:43       ` Marek Olšák
  2016-01-18 15:51         ` Jani Nikula
  2016-01-18 16:05         ` Emil Velikov
  0 siblings, 2 replies; 27+ messages in thread
From: Marek Olšák @ 2016-01-18 15:43 UTC (permalink / raw)
  To: Emil Velikov; +Cc: ML dri-devel

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] configure.ac: disable annoying warning -Wmissing-field-initializers
  2016-01-18 15:43       ` Marek Olšák
@ 2016-01-18 15:51         ` Jani Nikula
  2016-01-18 16:05         ` Emil Velikov
  1 sibling, 0 replies; 27+ messages in thread
From: Jani Nikula @ 2016-01-18 15:51 UTC (permalink / raw)
  To: Marek Olšák, Emil Velikov; +Cc: ML dri-devel

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

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] configure.ac: disable annoying warning -Wmissing-field-initializers
  2016-01-18 15:43       ` Marek Olšák
  2016-01-18 15:51         ` Jani Nikula
@ 2016-01-18 16:05         ` Emil Velikov
  2016-01-18 22:53           ` Marek Olšák
  2016-01-19  3:42           ` Michel Dänzer
  1 sibling, 2 replies; 27+ messages in thread
From: Emil Velikov @ 2016-01-18 16:05 UTC (permalink / raw)
  To: Marek Olšák; +Cc: ML 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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] configure.ac: disable annoying warning -Wmissing-field-initializers
  2016-01-18 16:05         ` Emil Velikov
@ 2016-01-18 22:53           ` Marek Olšák
  2016-01-21 10:51             ` Emil Velikov
  2016-01-19  3:42           ` Michel Dänzer
  1 sibling, 1 reply; 27+ messages in thread
From: Marek Olšák @ 2016-01-18 22:53 UTC (permalink / raw)
  To: Emil Velikov; +Cc: ML dri-devel

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] configure.ac: disable annoying warning -Wmissing-field-initializers
  2016-01-18 16:05         ` Emil Velikov
  2016-01-18 22:53           ` Marek Olšák
@ 2016-01-19  3:42           ` Michel Dänzer
  1 sibling, 0 replies; 27+ messages in thread
From: Michel Dänzer @ 2016-01-19  3:42 UTC (permalink / raw)
  To: Emil Velikov, Marek Olšák; +Cc: ML dri-devel

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.)


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] configure.ac: disable annoying warning -Wmissing-field-initializers
  2016-01-18 22:53           ` Marek Olšák
@ 2016-01-21 10:51             ` Emil Velikov
  2016-01-21 12:08               ` Marek Olšák
  0 siblings, 1 reply; 27+ messages in thread
From: Emil Velikov @ 2016-01-21 10:51 UTC (permalink / raw)
  To: Marek Olšák; +Cc: ML dri-devel

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] configure.ac: disable annoying warning -Wmissing-field-initializers
  2016-01-21 10:51             ` Emil Velikov
@ 2016-01-21 12:08               ` Marek Olšák
  2016-01-21 13:09                 ` Emil Velikov
  0 siblings, 1 reply; 27+ messages in thread
From: Marek Olšák @ 2016-01-21 12:08 UTC (permalink / raw)
  To: Emil Velikov; +Cc: ML dri-devel

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] configure.ac: disable annoying warning -Wmissing-field-initializers
  2016-01-21 12:08               ` Marek Olšák
@ 2016-01-21 13:09                 ` Emil Velikov
  2016-01-21 16:58                   ` Marek Olšák
  0 siblings, 1 reply; 27+ messages in thread
From: Emil Velikov @ 2016-01-21 13:09 UTC (permalink / raw)
  To: Marek Olšák; +Cc: ML dri-devel

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] configure.ac: disable annoying warning -Wmissing-field-initializers
  2016-01-21 13:09                 ` Emil Velikov
@ 2016-01-21 16:58                   ` Marek Olšák
  2016-01-22 17:13                     ` Emil Velikov
  0 siblings, 1 reply; 27+ messages in thread
From: Marek Olšák @ 2016-01-21 16:58 UTC (permalink / raw)
  To: Emil Velikov; +Cc: ML dri-devel

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] configure.ac: disable annoying warning -Wmissing-field-initializers
  2016-01-21 16:58                   ` Marek Olšák
@ 2016-01-22 17:13                     ` Emil Velikov
  2016-01-22 17:18                       ` Marek Olšák
  0 siblings, 1 reply; 27+ messages in thread
From: Emil Velikov @ 2016-01-22 17:13 UTC (permalink / raw)
  To: Marek Olšák; +Cc: ML dri-devel

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] configure.ac: disable annoying warning -Wmissing-field-initializers
  2016-01-22 17:13                     ` Emil Velikov
@ 2016-01-22 17:18                       ` Marek Olšák
  2016-01-22 17:29                         ` Ilia Mirkin
  0 siblings, 1 reply; 27+ messages in thread
From: Marek Olšák @ 2016-01-22 17:18 UTC (permalink / raw)
  To: Emil Velikov; +Cc: ML dri-devel

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] configure.ac: disable annoying warning -Wmissing-field-initializers
  2016-01-22 17:18                       ` Marek Olšák
@ 2016-01-22 17:29                         ` Ilia Mirkin
  2016-01-22 17:40                           ` Emil Velikov
  0 siblings, 1 reply; 27+ messages in thread
From: Ilia Mirkin @ 2016-01-22 17:29 UTC (permalink / raw)
  To: Marek Olšák; +Cc: Emil Velikov, ML dri-devel

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] configure.ac: disable annoying warning -Wmissing-field-initializers
  2016-01-22 17:29                         ` Ilia Mirkin
@ 2016-01-22 17:40                           ` Emil Velikov
  2016-01-22 17:47                             ` Ville Syrjälä
  0 siblings, 1 reply; 27+ messages in thread
From: Emil Velikov @ 2016-01-22 17:40 UTC (permalink / raw)
  To: Ilia Mirkin; +Cc: ML dri-devel

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] configure.ac: disable annoying warning -Wmissing-field-initializers
  2016-01-22 17:40                           ` Emil Velikov
@ 2016-01-22 17:47                             ` Ville Syrjälä
  2016-01-22 17:48                               ` Ilia Mirkin
  2016-01-22 17:50                               ` Emil Velikov
  0 siblings, 2 replies; 27+ messages in thread
From: Ville Syrjälä @ 2016-01-22 17:47 UTC (permalink / raw)
  To: Emil Velikov; +Cc: ML dri-devel

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?

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] configure.ac: disable annoying warning -Wmissing-field-initializers
  2016-01-22 17:47                             ` Ville Syrjälä
@ 2016-01-22 17:48                               ` Ilia Mirkin
  2016-01-22 19:18                                 ` Jan Vesely
  2016-01-22 17:50                               ` Emil Velikov
  1 sibling, 1 reply; 27+ messages in thread
From: Ilia Mirkin @ 2016-01-22 17:48 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: Emil Velikov, ML dri-devel

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] configure.ac: disable annoying warning -Wmissing-field-initializers
  2016-01-22 17:47                             ` Ville Syrjälä
  2016-01-22 17:48                               ` Ilia Mirkin
@ 2016-01-22 17:50                               ` Emil Velikov
  2016-01-22 17:59                                 ` Emil Velikov
  1 sibling, 1 reply; 27+ messages in thread
From: Emil Velikov @ 2016-01-22 17:50 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: ML dri-devel

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] configure.ac: disable annoying warning -Wmissing-field-initializers
  2016-01-22 17:50                               ` Emil Velikov
@ 2016-01-22 17:59                                 ` Emil Velikov
  2016-01-22 18:49                                   ` Ville Syrjälä
  0 siblings, 1 reply; 27+ messages in thread
From: Emil Velikov @ 2016-01-22 17:59 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: ML dri-devel

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] configure.ac: disable annoying warning -Wmissing-field-initializers
  2016-01-22 17:59                                 ` Emil Velikov
@ 2016-01-22 18:49                                   ` Ville Syrjälä
  2016-01-22 19:21                                     ` Emil Velikov
  0 siblings, 1 reply; 27+ messages in thread
From: Ville Syrjälä @ 2016-01-22 18:49 UTC (permalink / raw)
  To: Emil Velikov; +Cc: ML dri-devel

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.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] configure.ac: disable annoying warning -Wmissing-field-initializers
  2016-01-22 17:48                               ` Ilia Mirkin
@ 2016-01-22 19:18                                 ` Jan Vesely
  0 siblings, 0 replies; 27+ messages in thread
From: Jan Vesely @ 2016-01-22 19:18 UTC (permalink / raw)
  To: Ilia Mirkin, Ville Syrjälä; +Cc: Emil Velikov, ML dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 3599 bytes --]

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
-- 
Jan Vesely <jan.vesely@rutgers.edu>

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] configure.ac: disable annoying warning -Wmissing-field-initializers
  2016-01-22 18:49                                   ` Ville Syrjälä
@ 2016-01-22 19:21                                     ` Emil Velikov
  2016-01-22 19:33                                       ` Ville Syrjälä
  0 siblings, 1 reply; 27+ messages in thread
From: Emil Velikov @ 2016-01-22 19:21 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: ML 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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH] configure.ac: disable annoying warning -Wmissing-field-initializers
  2016-01-22 19:21                                     ` Emil Velikov
@ 2016-01-22 19:33                                       ` Ville Syrjälä
  0 siblings, 0 replies; 27+ messages in thread
From: Ville Syrjälä @ 2016-01-22 19:33 UTC (permalink / raw)
  To: Emil Velikov; +Cc: ML dri-devel

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.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2016-01-22 19:33 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-12 21:14 [PATCH] configure.ac: disable annoying warning -Wmissing-field-initializers Marek Olšák
2016-01-15 11:12 ` Emil Velikov
2016-01-15 15:24   ` Marek Olšák
2016-01-16 19:46     ` David Herrmann
2016-01-18 14:45     ` Emil Velikov
2016-01-18 15:43       ` Marek Olšák
2016-01-18 15:51         ` Jani Nikula
2016-01-18 16:05         ` Emil Velikov
2016-01-18 22:53           ` Marek Olšák
2016-01-21 10:51             ` Emil Velikov
2016-01-21 12:08               ` Marek Olšák
2016-01-21 13:09                 ` Emil Velikov
2016-01-21 16:58                   ` Marek Olšák
2016-01-22 17:13                     ` Emil Velikov
2016-01-22 17:18                       ` Marek Olšák
2016-01-22 17:29                         ` Ilia Mirkin
2016-01-22 17:40                           ` Emil Velikov
2016-01-22 17:47                             ` Ville Syrjälä
2016-01-22 17:48                               ` Ilia Mirkin
2016-01-22 19:18                                 ` Jan Vesely
2016-01-22 17:50                               ` Emil Velikov
2016-01-22 17:59                                 ` Emil Velikov
2016-01-22 18:49                                   ` Ville Syrjälä
2016-01-22 19:21                                     ` Emil Velikov
2016-01-22 19:33                                       ` Ville Syrjälä
2016-01-19  3:42           ` Michel Dänzer
2016-01-16 19:49 ` Ilia Mirkin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.