Re: [PATCH][next] media: venus: hfi_cmds: Replace fake flex-array with flexible-array member

[Konrad Dybcio <konrad.dybcio@linaro.org>]

On 17.05.2023 04:11, Konrad Dybcio wrote:
> 
> 
> On 17.05.2023 03:03, Gustavo A. R. Silva wrote:
>> One-element arrays are deprecated, and we are replacing them with flexible
>> array members instead. So, replace one-element arrays with flexible-array
>> members in struct hfi_sys_set_resource_pkt, and refactor the rest of
>> the code, accordingly.
>>
>> This helps with the ongoing efforts to tighten the FORTIFY_SOURCE
>> routines on memcpy() and help us make progress towards globally
>> enabling -fstrict-flex-arrays=3 [1].
>>
>> The only binary differences seen before/after changes are the
>> following:
>>
>>      17ba:      mov    %rbx,%rdi
>>      17bd:      call   17c2 <pkt_sys_set_resource+0x42>
>>                         17be: R_X86_64_PLT32    __tsan_write4-0x4
>> -    17c2:      movl   $0x14,(%rbx)
>> +    17c2:      movl   $0x10,(%rbx)
>>      17c8:      lea    0x4(%rbx),%rdi
>>      17cc:      call   17d1 <pkt_sys_set_resource+0x51>
>>                         17cd: R_X86_64_PLT32    __tsan_write4-0x4
>>
>> which is expected once this accounts for the following line of code
>> at  drivers/media/platform/qcom/venus/hfi_cmds.c:73
>>
>> 73         pkt->hdr.size = sizeof(*pkt);
>>
>> and as *pkt is of type struct hfi_sys_set_resource_pkt, sizeof(*pkt) is
>> reduced by 4 bytes, due to the flex-array transformation.
>>
>> Link: https://github.com/KSPP/linux/issues/79
>> Link: https://github.com/KSPP/linux/issues/293
>> Link: https://gcc.gnu.org/pipermail/gcc-patches/2022-October/602902.html [1]
>> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
>> ---
>>  drivers/media/platform/qcom/venus/hfi_cmds.c | 2 +-
>>  drivers/media/platform/qcom/venus/hfi_cmds.h | 2 +-
>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/media/platform/qcom/venus/hfi_cmds.c b/drivers/media/platform/qcom/venus/hfi_cmds.c
>> index 3f74d518ad08..7c82e212434e 100644
>> --- a/drivers/media/platform/qcom/venus/hfi_cmds.c
>> +++ b/drivers/media/platform/qcom/venus/hfi_cmds.c
>> @@ -83,7 +83,7 @@ int pkt_sys_set_resource(struct hfi_sys_set_resource_pkt *pkt, u32 id, u32 size,
>>  		res->size = size;
>>  		res->mem = addr;
>>  		pkt->resource_type = HFI_RESOURCE_OCMEM;
>> -		pkt->hdr.size += sizeof(*res) - sizeof(u32);
>> +		pkt->hdr.size += sizeof(*res);
>>  		break;
>>  	}
>>  	case VIDC_RESOURCE_NONE:
>> diff --git a/drivers/media/platform/qcom/venus/hfi_cmds.h b/drivers/media/platform/qcom/venus/hfi_cmds.h
>> index ba74d03eb9cd..dd9c5066442d 100644
>> --- a/drivers/media/platform/qcom/venus/hfi_cmds.h
>> +++ b/drivers/media/platform/qcom/venus/hfi_cmds.h
>> @@ -56,7 +56,7 @@ struct hfi_sys_set_resource_pkt {
>>  	struct hfi_pkt_hdr hdr;
>>  	u32 resource_handle;
>>  	u32 resource_type;
>> -	u32 resource_data[1];
>> +	u32 resource_data[];
> Would making this an u32* be a better resolution?
Nevermind, I overthought this by thinking in the terms of its size
and not the data within the struct...

Maybe struct_size could be used instead of subtracting sizeof(u32)
though?

Konrad
> 
> Konrad
>>  };
>>  
>>  struct hfi_sys_release_resource_pkt {