[PATCH] drm/amdgpu/gmc: fix compiler errors [-Werror,-Wmissing-braces]

[PATCH] drm/amdgpu/gmc: fix compiler errors [-Werror,-Wmissing-braces]

[S, Shirish]

Initializing structures with { } is known to be problematic since
it doesn't necessararily initializes all bytes, in case of padding,
causing random failures when structures are memcmp().

This patch fixes the structure initialisation compiler error by memsetting
the entire structure elements instead of only the first one.

Signed-off-by: Shirish S <shirish.s@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c | 3 ++-
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 3 ++-
 2 files changed, 4 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 1ad7e6b..3444067 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
@@ -1471,8 +1471,9 @@ 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;
 
+		memset(&task_info, 0, sizeof(struct amdgpu_task_info));
 		amdgpu_vm_get_task_info(adev, entry->pasid, &task_info);
 
 		dev_err(adev->dev, "GPU fault detected: %d 0x%08x for process %s pid %d thread %s pid %d\n",
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index bacdaef..2bfddce 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -320,8 +320,9 @@ 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;
 
+		memset(&task_info, 0, sizeof(amdgpu_task_info));
 		amdgpu_vm_get_task_info(adev, entry->pasid, &task_info);
 
 		dev_err(adev->dev,
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu/gmc: fix compiler errors [-Werror,-Wmissing-braces]

[Deucher, Alexander]

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

Reviewed-by: Alex Deucher <alexander.deucher-5C7GfCeVMHo@public.gmane.org>

________________________________
From: amd-gfx <amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org> on behalf of S, Shirish <Shirish.S-5C7GfCeVMHo@public.gmane.org>
Sent: Thursday, December 20, 2018 9:01:33 AM
To: Li, Sun peng (Leo); Wentland, Harry; Deucher, Alexander; Koenig, Christian
Cc: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org; S, Shirish
Subject: [PATCH] drm/amdgpu/gmc: fix compiler errors [-Werror,-Wmissing-braces]

Initializing structures with { } is known to be problematic since
it doesn't necessararily initializes all bytes, in case of padding,
causing random failures when structures are memcmp().

This patch fixes the structure initialisation compiler error by memsetting
the entire structure elements instead of only the first one.

Signed-off-by: Shirish S <shirish.s-5C7GfCeVMHo@public.gmane.org>
---
 drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c | 3 ++-
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 3 ++-
 2 files changed, 4 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 1ad7e6b..3444067 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
@@ -1471,8 +1471,9 @@ 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;

+               memset(&task_info, 0, sizeof(struct amdgpu_task_info));
                 amdgpu_vm_get_task_info(adev, entry->pasid, &task_info);

                 dev_err(adev->dev, "GPU fault detected: %d 0x%08x for process %s pid %d thread %s pid %d\n",
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index bacdaef..2bfddce 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -320,8 +320,9 @@ 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;

+               memset(&task_info, 0, sizeof(amdgpu_task_info));
                 amdgpu_vm_get_task_info(adev, entry->pasid, &task_info);

                 dev_err(adev->dev,
--
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[-- Attachment #1.2: Type: text/html, Size: 4840 bytes --]

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

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu/gmc: fix compiler errors [-Werror,-Wmissing-braces]

[Michel Dänzer]

On 2018-12-20 3:01 p.m., S, Shirish wrote:
> Initializing structures with { } is known to be problematic since
> it doesn't necessararily initializes all bytes, in case of padding,
> causing random failures when structures are memcmp().
> 
> This patch fixes the structure initialisation compiler error by memsetting
> the entire structure elements instead of only the first one.
> 
> Signed-off-by: Shirish S <shirish.s@amd.com>
> 
> [...]
>  
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> index bacdaef..2bfddce 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> @@ -320,8 +320,9 @@ 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;
>  
> +		memset(&task_info, 0, sizeof(amdgpu_task_info));

This doesn't compile:

drivers/gpu/drm//amd/amdgpu/gmc_v9_0.c: In function ‘gmc_v9_0_process_interrupt’:
drivers/gpu/drm//amd/amdgpu/gmc_v9_0.c:325:32: error: ‘amdgpu_task_info’ undeclared (first use in this function)
   memset(&task_info, 0, sizeof(amdgpu_task_info));
                                ^~~~~~~~~~~~~~~~

(Needs to be "sizeof(struct amdgpu_task_info)")


I've reverted this on the internal amd-staging-drm-next branch.

Patches must be tested appropriately (against the target branch),
ideally before submitting them for review. Reviewers are not expected to
look for things which trivially break the build.


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

Re: [PATCH] drm/amdgpu/gmc: fix compiler errors [-Werror,-Wmissing-braces]

[Koenig, Christian]

Am 21.12.18 um 12:18 schrieb Michel Dänzer:
> On 2018-12-20 3:01 p.m., S, Shirish wrote:
>> Initializing structures with { } is known to be problematic since
>> it doesn't necessararily initializes all bytes, in case of padding,
>> causing random failures when structures are memcmp().
>>
>> This patch fixes the structure initialisation compiler error by memsetting
>> the entire structure elements instead of only the first one.
>>
>> Signed-off-by: Shirish S <shirish.s@amd.com>
>>
>> [...]
>>   
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> index bacdaef..2bfddce 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> @@ -320,8 +320,9 @@ 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;
>>   
>> +		memset(&task_info, 0, sizeof(amdgpu_task_info));
> This doesn't compile:
>
> drivers/gpu/drm//amd/amdgpu/gmc_v9_0.c: In function ‘gmc_v9_0_process_interrupt’:
> drivers/gpu/drm//amd/amdgpu/gmc_v9_0.c:325:32: error: ‘amdgpu_task_info’ undeclared (first use in this function)
>     memset(&task_info, 0, sizeof(amdgpu_task_info));
>                                  ^~~~~~~~~~~~~~~~
>
> (Needs to be "sizeof(struct amdgpu_task_info)")

The coding style guide lines actually document that you should avoid 
types with sizeof and instead use the variables directly.

E.g. use "memset(&task_info, 0, sizeof(task_info));"

> I've reverted this on the internal amd-staging-drm-next branch.
>
> Patches must be tested appropriately (against the target branch),
> ideally before submitting them for review. Reviewers are not expected to
> look for things which trivially break the build.

Completely agree.

I mean I break things more often than not myself because of our limited 
QA resources, but at least such trivial things should not happen any more.

Regards,
Christian.
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu/gmc: fix compiler errors [-Werror,-Wmissing-braces]

[S]

Have rectified it.

I had built it but seems some how i missed it while rebasing the patch 
from my internal kernel tree (which is a bit older) onto 
internal-staging tip.

Apologies for the troubles.

Have sent V2, please review.

Regards,

Shirish S

On 12/21/2018 4:48 PM, Michel Dänzer wrote:
> On 2018-12-20 3:01 p.m., S, Shirish wrote:
>> Initializing structures with { } is known to be problematic since
>> it doesn't necessararily initializes all bytes, in case of padding,
>> causing random failures when structures are memcmp().
>>
>> This patch fixes the structure initialisation compiler error by memsetting
>> the entire structure elements instead of only the first one.
>>
>> Signed-off-by: Shirish S <shirish.s@amd.com>
>>
>> [...]
>>   
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> index bacdaef..2bfddce 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> @@ -320,8 +320,9 @@ 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;
>>   
>> +		memset(&task_info, 0, sizeof(amdgpu_task_info));
> This doesn't compile:
>
> drivers/gpu/drm//amd/amdgpu/gmc_v9_0.c: In function ‘gmc_v9_0_process_interrupt’:
> drivers/gpu/drm//amd/amdgpu/gmc_v9_0.c:325:32: error: ‘amdgpu_task_info’ undeclared (first use in this function)
>     memset(&task_info, 0, sizeof(amdgpu_task_info));
>                                  ^~~~~~~~~~~~~~~~
>
> (Needs to be "sizeof(struct amdgpu_task_info)")
>
>
> I've reverted this on the internal amd-staging-drm-next branch.
>
> Patches must be tested appropriately (against the target branch),
> ideally before submitting them for review. Reviewers are not expected to
> look for things which trivially break the build.
>
>
-- 
Regards,
Shirish S

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx