On 18 January 2016 at 17:43, Marek Olšák <maraeo@xxxxxxxxx> wrote:
> On Mon, Jan 18, 2016 at 3:45 PM, Emil Velikov <emil.l.velikov@xxxxxxxxx> wrote:
>> On 15 January 2016 at 17:24, Marek Olšák <maraeo@xxxxxxxxx> wrote:
>>> On Fri, Jan 15, 2016 at 12:12 PM, Emil Velikov <emil.l.velikov@xxxxxxxxx> wrote:
>>>> On 12 January 2016 at 23:14, Marek Olšák <maraeo@xxxxxxxxx> wrote:
>>>>> From: Marek Olšák <marek.olsak@xxxxxxx>
>>>>>
>>>>> 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@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
