linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Guenter Roeck <linux@roeck-us.net>
To: christian.koenig@amd.com, Peng Hao <peng.hao2@zte.com.cn>
Cc: "airlied@linux.ie" <airlied@linux.ie>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>,
	"amd-gfx@lists.freedesktop.org" <amd-gfx@lists.freedesktop.org>,
	"Deucher, Alexander" <Alexander.Deucher@amd.com>
Subject: Re: [PATCH] amdgpu/gmc : fix compile warning
Date: Mon, 8 Oct 2018 06:33:01 -0700	[thread overview]
Message-ID: <c72a23ea-efea-4c0e-d397-623d6ac16ebb@roeck-us.net> (raw)
In-Reply-To: <4da23fcb-4a94-2695-ad80-929025e84bd2@gmail.com>

On 10/08/2018 01:00 AM, Christian König wrote:
> Am 05.10.2018 um 10:38 schrieb Guenter Roeck:
>> On 10/05/2018 01:14 AM, Koenig, Christian wrote:
>>> Am 04.10.2018 um 20:52 schrieb Guenter Roeck:
>>>> Hi,
>>>>
>>>> On Fri, Sep 14, 2018 at 06:05:52PM +0800, Peng Hao wrote:
>>>>> drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c:
>>>>>       In function ‘gmc_v8_0_process_interrupt’:
>>>>> drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c:1447:10:
>>>>>       warning: missing braces around initializer [-Wmissing-braces]
>>>>>
>>>>> Signed-off-by: Peng Hao <peng.hao2@zte.com.cn>
>>>> Was there any feedback on this patch ? The problem does affect us,
>>>> and we'll need a fix.
>>>
>>> Well as discussed using "{ { 0 } }" is as wrong as using "{ 0 }".
>>>
>>
>> Ah, sorry, I must have missed the discussion.
>>
>> It is for sure not the best solution, but at least it compiles, and it seems
>> to be proliferating.
> 
> Yeah, and exactly that's the problem. As the discussion showed "{ { 0 } }" is buggy because it tells the compiler to only initialize the first member of the structure, but not all of it.
> 
> That is incorrect and rather dangerous cause it can lead to unforeseen results and should probably trigger a warning.
> 
>>
>> $ git grep "{ *{ *0 *} *}" | wc
>>     144    1180   11802
>> $ git grep "{ *{ *0 *} *}" drivers/gpu/drm/amd/ | wc
>>      50     459    5239
>>
>>> We should either use only "{ }" or even better make nails with heads and
>>> use memset().
>>
>> I'd rather leave it up to the compiler to decide what is most efficient.
> 
> And I would rather prefer to have a working driver :)
> 

So { } isn't correct either ?

One thing I found missing in the discussion was the reference to the C standard.
The C99 standard states in section 6.7.8 (Initialization) clause 19: "... all
subobjects that are not initialized explicitly shall be initialized implicitly
the same as objects that have static storage duration". Clause 21 makes further
reference to partial initialization, suggesting the same. Various online
resources, including the gcc documentation, all state the same. I don't find
any reference to a partial initialization which would leave members of a structure
undefined. It would be interesting for me to understand how and why this does
not apply here.

In this context, it is interesting that the other 48 instances of the
{ { 0 } } initialization in the same driver don't raise similar concerns,
nor seemed to have caused any operational problems.

Anyway, I fixed up the code in our tree (with { }), so I'll leave it
up to you folks to decide what if anything to do about it.

Thanks,
Guenter

  reply	other threads:[~2018-10-08 13:33 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-14 10:05 Peng Hao
2018-10-04 18:52 ` Guenter Roeck
2018-10-05  8:14   ` Koenig, Christian
2018-10-05  8:38     ` Guenter Roeck
2018-10-08  8:00       ` Christian König
2018-10-08 13:33         ` Guenter Roeck [this message]
2018-10-08 13:47           ` Koenig, Christian
2018-10-08 14:10             ` Guenter Roeck
2018-10-08 15:57               ` Deucher, Alexander
2018-10-08 17:22                 ` Koenig, Christian
2018-10-08 17:46                   ` Guenter Roeck
2018-10-08 18:13                     ` Koenig, Christian
2018-10-19  8:53                       ` Daniel Vetter
2018-10-19  8:56                         ` Daniel Vetter
2018-10-19 13:08                         ` Guenter Roeck
2018-10-19 15:30                           ` Alex Deucher
2018-10-08 17:41                 ` Guenter Roeck
2018-10-08 18:24                   ` Deucher, Alexander

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c72a23ea-efea-4c0e-d397-623d6ac16ebb@roeck-us.net \
    --to=linux@roeck-us.net \
    --cc=Alexander.Deucher@amd.com \
    --cc=airlied@linux.ie \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=christian.koenig@amd.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peng.hao2@zte.com.cn \
    --subject='Re: [PATCH] amdgpu/gmc : fix compile warning' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).