From: "Christian König" <ckoenig.leichtzumerken@gmail.com>
To: Guenter Roeck <linux@roeck-us.net>,
"Koenig, Christian" <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 10:00:15 +0200 [thread overview]
Message-ID: <4da23fcb-4a94-2695-ad80-929025e84bd2@gmail.com> (raw)
In-Reply-To: <4772f72c-6018-3556-6324-5f49faa00073@roeck-us.net>
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 :)
Christian.
>
> Guenter
>
>>
>> Christian.
>>
>>>
>>>> ---
>>>> drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c | 2 +-
>>>> drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 2 +-
>>>> 2 files changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>>>> b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>>>> index 9333109..55f4755 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>>>> @@ -1444,7 +1444,7 @@ static int gmc_v8_0_process_interrupt(struct
>>>> amdgpu_device *adev,
>>>> gmc_v8_0_set_fault_enable_default(adev, false);
>>>> if (printk_ratelimit()) {
>>>> - struct amdgpu_task_info task_info = { 0 };
>>>> + struct amdgpu_task_info task_info = { {0} };
>>> I wondered if
>>> struct amdgpu_task_info task_info = { };
>>> would be better.
>>>
>>> Thanks,
>>> Guenter
>>>
>>>> amdgpu_vm_get_task_info(adev, entry->pasid, &task_info);
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>> index 72f8018..e8b78c5 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>> @@ -259,7 +259,7 @@ static int gmc_v9_0_process_interrupt(struct
>>>> amdgpu_device *adev,
>>>> }
>>>> if (printk_ratelimit()) {
>>>> - struct amdgpu_task_info task_info = { 0 };
>>>> + struct amdgpu_task_info task_info = { {0} };
>>>> amdgpu_vm_get_task_info(adev, entry->pasid, &task_info);
>>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2018-10-08 8:00 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-09-14 10:05 [PATCH] amdgpu/gmc : fix compile warning 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 [this message]
2018-10-08 13:33 ` Guenter Roeck
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=4da23fcb-4a94-2695-ad80-929025e84bd2@gmail.com \
--to=ckoenig.leichtzumerken@gmail.com \
--cc=Alexander.Deucher@amd.com \
--cc=Christian.Koenig@amd.com \
--cc=airlied@linux.ie \
--cc=amd-gfx@lists.freedesktop.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@roeck-us.net \
--cc=peng.hao2@zte.com.cn \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).