Re: [PATCH] amdgpu/gmc : fix compile warning

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux