2022-02-18 03:09:59

by CGEL

[permalink] [raw]
Subject: [PATCH] drm/amdkfd: Replace one-element array with flexible-array member

From: Changcheng Deng <[email protected]>

There is a regular need in the kernel to provide a way to declare having
a dynamically sized set of trailing elements in a structure. Kernel code
should always use "flexible array members" for these cases. The older
style of one-element or zero-length arrays should no longer be used.
Reference:
https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays

Reported-by: Zeal Robot <[email protected]>
Signed-off-by: Changcheng Deng <[email protected]>
---
drivers/gpu/drm/amd/amdkfd/kfd_crat.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_crat.h b/drivers/gpu/drm/amd/amdkfd/kfd_crat.h
index 482ba84a728d..d7c38fbc533e 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_crat.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_crat.h
@@ -310,7 +310,7 @@ struct cdit_header {
uint32_t creator_revision;
uint32_t total_entries;
uint16_t num_domains;
- uint8_t entry[1];
+ uint8_t entry[];
};

#pragma pack()
--
2.25.1


2022-02-18 08:10:36

by Christian König

[permalink] [raw]
Subject: Re: [PATCH] drm/amdkfd: Replace one-element array with flexible-array member

Felix need to comment as well, but I don't think that this will work
that easily.

By changing the entry from 1 to 0 your are also changing the size of the
structure.

Regards,
Christian.

Am 18.02.22 um 04:09 schrieb [email protected]:
> From: Changcheng Deng <[email protected]>
>
> There is a regular need in the kernel to provide a way to declare having
> a dynamically sized set of trailing elements in a structure. Kernel code
> should always use "flexible array members" for these cases. The older
> style of one-element or zero-length arrays should no longer be used.
> Reference:
> https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays
>
> Reported-by: Zeal Robot <[email protected]>
> Signed-off-by: Changcheng Deng <[email protected]>
> ---
> drivers/gpu/drm/amd/amdkfd/kfd_crat.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_crat.h b/drivers/gpu/drm/amd/amdkfd/kfd_crat.h
> index 482ba84a728d..d7c38fbc533e 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_crat.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_crat.h
> @@ -310,7 +310,7 @@ struct cdit_header {
> uint32_t creator_revision;
> uint32_t total_entries;
> uint16_t num_domains;
> - uint8_t entry[1];
> + uint8_t entry[];
> };
>
> #pragma pack()

2022-02-18 21:31:38

by Felix Kuehling

[permalink] [raw]
Subject: Re: [PATCH] drm/amdkfd: Replace one-element array with flexible-array member

It looks like this structure isn't being used at all. So I'm OK with
this change, in case we ever use it in the future.

Regards,
  Felix


Am 2022-02-18 um 02:47 schrieb Christian König:
> Felix need to comment as well, but I don't think that this will work
> that easily.
>
> By changing the entry from 1 to 0 your are also changing the size of
> the structure.
>
> Regards,
> Christian.
>
> Am 18.02.22 um 04:09 schrieb [email protected]:
>> From: Changcheng Deng <[email protected]>
>>
>> There is a regular need in the kernel to provide a way to declare having
>> a dynamically sized set of trailing elements in a structure. Kernel code
>> should always use "flexible array members" for these cases. The older
>> style of one-element or zero-length arrays should no longer be used.
>> Reference:
>> https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays
>>
>>
>> Reported-by: Zeal Robot <[email protected]>
>> Signed-off-by: Changcheng Deng <[email protected]>
>> ---
>>   drivers/gpu/drm/amd/amdkfd/kfd_crat.h | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_crat.h
>> b/drivers/gpu/drm/amd/amdkfd/kfd_crat.h
>> index 482ba84a728d..d7c38fbc533e 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_crat.h
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_crat.h
>> @@ -310,7 +310,7 @@ struct cdit_header {
>>       uint32_t    creator_revision;
>>       uint32_t    total_entries;
>>       uint16_t    num_domains;
>> -    uint8_t        entry[1];
>> +    uint8_t        entry[];
>>   };
>>     #pragma pack()
>

2022-02-20 09:59:24

by Christian König

[permalink] [raw]
Subject: Re: [PATCH] drm/amdkfd: Replace one-element array with flexible-array member

Well in that case somebody could argue that we should probably remove
the structure altogether.

Regards,
Christian.

Am 18.02.22 um 17:15 schrieb Felix Kuehling:
> It looks like this structure isn't being used at all. So I'm OK with
> this change, in case we ever use it in the future.
>
> Regards,
>   Felix
>
>
> Am 2022-02-18 um 02:47 schrieb Christian König:
>> Felix need to comment as well, but I don't think that this will work
>> that easily.
>>
>> By changing the entry from 1 to 0 your are also changing the size of
>> the structure.
>>
>> Regards,
>> Christian.
>>
>> Am 18.02.22 um 04:09 schrieb [email protected]:
>>> From: Changcheng Deng <[email protected]>
>>>
>>> There is a regular need in the kernel to provide a way to declare
>>> having
>>> a dynamically sized set of trailing elements in a structure. Kernel
>>> code
>>> should always use "flexible array members" for these cases. The older
>>> style of one-element or zero-length arrays should no longer be used.
>>> Reference:
>>> https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays
>>>
>>>
>>> Reported-by: Zeal Robot <[email protected]>
>>> Signed-off-by: Changcheng Deng <[email protected]>
>>> ---
>>>   drivers/gpu/drm/amd/amdkfd/kfd_crat.h | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_crat.h
>>> b/drivers/gpu/drm/amd/amdkfd/kfd_crat.h
>>> index 482ba84a728d..d7c38fbc533e 100644
>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_crat.h
>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_crat.h
>>> @@ -310,7 +310,7 @@ struct cdit_header {
>>>       uint32_t    creator_revision;
>>>       uint32_t    total_entries;
>>>       uint16_t    num_domains;
>>> -    uint8_t        entry[1];
>>> +    uint8_t        entry[];
>>>   };
>>>     #pragma pack()
>>