diff mbox series

[4/4] wifi: ath11k: Use DECLARE_FLEX_ARRAY() for ath11k_htc_record

Message ID 20231127-flexarray-htc_record-v1-4-6be1f36126fd@quicinc.com (mailing list archive)
State Superseded
Delegated to: Kalle Valo
Headers show
Series wifi: ath*: use DECLARE_FLEX_ARRAY() for ath*_htc_record | expand

Commit Message

Jeff Johnson Nov. 27, 2023, 4:14 p.m. UTC
Transform the zero-length array in ath11k_htc_record into a proper
flexible array via the DECLARE_FLEX_ARRAY() macro. This helps with
ongoing efforts to globally enable -Warray-bounds.

Signed-off-by: Jeff Johnson <quic_jjohnson@quicinc.com>
---
 drivers/net/wireless/ath/ath11k/htc.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Gustavo A. R. Silva Nov. 27, 2023, 4:23 p.m. UTC | #1
On 11/27/23 10:14, Jeff Johnson wrote:
> Transform the zero-length array in ath11k_htc_record into a proper
> flexible array via the DECLARE_FLEX_ARRAY() macro. This helps with
> ongoing efforts to globally enable -Warray-bounds.
> 
> Signed-off-by: Jeff Johnson <quic_jjohnson@quicinc.com>
> ---
>   drivers/net/wireless/ath/ath11k/htc.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/ath/ath11k/htc.h b/drivers/net/wireless/ath/ath11k/htc.h
> index 84971cc9251c..e0434b29df70 100644
> --- a/drivers/net/wireless/ath/ath11k/htc.h
> +++ b/drivers/net/wireless/ath/ath11k/htc.h
> @@ -151,7 +151,7 @@ struct ath11k_htc_credit_report {
>   struct ath11k_htc_record {
>   	struct ath11k_htc_record_hdr hdr;
>   	union {
> -		struct ath11k_htc_credit_report credit_report[0];
> +		DECLARE_FLEX_ARRAY(struct ath11k_htc_credit_report, credit_report);
>   	};

Why not removing the `union` and just do a direct transformation [0] -> [ ] ?

--
Gustavo
Jeff Johnson Nov. 27, 2023, 4:32 p.m. UTC | #2
On 11/27/2023 8:23 AM, Gustavo A. R. Silva wrote:
> 
> 
> On 11/27/23 10:14, Jeff Johnson wrote:
>> Transform the zero-length array in ath11k_htc_record into a proper
>> flexible array via the DECLARE_FLEX_ARRAY() macro. This helps with
>> ongoing efforts to globally enable -Warray-bounds.
>>
>> Signed-off-by: Jeff Johnson <quic_jjohnson@quicinc.com>
>> ---
>>   drivers/net/wireless/ath/ath11k/htc.h | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath11k/htc.h b/drivers/net/wireless/ath/ath11k/htc.h
>> index 84971cc9251c..e0434b29df70 100644
>> --- a/drivers/net/wireless/ath/ath11k/htc.h
>> +++ b/drivers/net/wireless/ath/ath11k/htc.h
>> @@ -151,7 +151,7 @@ struct ath11k_htc_credit_report {
>>   struct ath11k_htc_record {
>>   	struct ath11k_htc_record_hdr hdr;
>>   	union {
>> -		struct ath11k_htc_credit_report credit_report[0];
>> +		DECLARE_FLEX_ARRAY(struct ath11k_htc_credit_report, credit_report);
>>   	};
> 
> Why not removing the `union` and just do a direct transformation [0] -> [ ] ?

No reason other than staying consistent with ath10k.
Will see if Kalle has an opinion on this.

/jeff
Kalle Valo Dec. 5, 2023, 3:29 p.m. UTC | #3
Jeff Johnson <quic_jjohnson@quicinc.com> writes:

> On 11/27/2023 8:23 AM, Gustavo A. R. Silva wrote:
>
>> 
>> 
>> On 11/27/23 10:14, Jeff Johnson wrote:
>>> Transform the zero-length array in ath11k_htc_record into a proper
>>> flexible array via the DECLARE_FLEX_ARRAY() macro. This helps with
>>> ongoing efforts to globally enable -Warray-bounds.
>>>
>>> Signed-off-by: Jeff Johnson <quic_jjohnson@quicinc.com>
>>> ---
>>>   drivers/net/wireless/ath/ath11k/htc.h | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/wireless/ath/ath11k/htc.h b/drivers/net/wireless/ath/ath11k/htc.h
>>> index 84971cc9251c..e0434b29df70 100644
>>> --- a/drivers/net/wireless/ath/ath11k/htc.h
>>> +++ b/drivers/net/wireless/ath/ath11k/htc.h
>>> @@ -151,7 +151,7 @@ struct ath11k_htc_credit_report {
>>>   struct ath11k_htc_record {
>>>   	struct ath11k_htc_record_hdr hdr;
>>>   	union {
>>> -		struct ath11k_htc_credit_report credit_report[0];
>>> +		DECLARE_FLEX_ARRAY(struct ath11k_htc_credit_report, credit_report);
>>>   	};
>> 
>> Why not removing the `union` and just do a direct transformation [0] -> [ ] ?
>
> No reason other than staying consistent with ath10k.
> Will see if Kalle has an opinion on this.

Yeah, I don't see the need for the union and I removed it in the pending
branch:

https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=a2faeea1fe0635563187e7821a6d0baf7b40f2c6

Does it look ok?
Gustavo A. R. Silva Dec. 5, 2023, 3:45 p.m. UTC | #4
On 12/5/23 09:29, Kalle Valo wrote:
> Jeff Johnson <quic_jjohnson@quicinc.com> writes:
> 
>> On 11/27/2023 8:23 AM, Gustavo A. R. Silva wrote:
>>
>>>
>>>
>>> On 11/27/23 10:14, Jeff Johnson wrote:
>>>> Transform the zero-length array in ath11k_htc_record into a proper
>>>> flexible array via the DECLARE_FLEX_ARRAY() macro. This helps with
>>>> ongoing efforts to globally enable -Warray-bounds.
>>>>
>>>> Signed-off-by: Jeff Johnson <quic_jjohnson@quicinc.com>
>>>> ---
>>>>    drivers/net/wireless/ath/ath11k/htc.h | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/net/wireless/ath/ath11k/htc.h b/drivers/net/wireless/ath/ath11k/htc.h
>>>> index 84971cc9251c..e0434b29df70 100644
>>>> --- a/drivers/net/wireless/ath/ath11k/htc.h
>>>> +++ b/drivers/net/wireless/ath/ath11k/htc.h
>>>> @@ -151,7 +151,7 @@ struct ath11k_htc_credit_report {
>>>>    struct ath11k_htc_record {
>>>>    	struct ath11k_htc_record_hdr hdr;
>>>>    	union {
>>>> -		struct ath11k_htc_credit_report credit_report[0];
>>>> +		DECLARE_FLEX_ARRAY(struct ath11k_htc_credit_report, credit_report);
>>>>    	};
>>>
>>> Why not removing the `union` and just do a direct transformation [0] -> [ ] ?
>>
>> No reason other than staying consistent with ath10k.
>> Will see if Kalle has an opinion on this.
> 
> Yeah, I don't see the need for the union and I removed it in the pending
> branch:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=a2faeea1fe0635563187e7821a6d0baf7b40f2c6
> 
> Does it look ok?
> 

Nope.

A direct transformation is just fine:

-	union {
-		struct ath11k_htc_credit_report credit_report[0];
-	};
+	struct ath11k_htc_credit_report credit_report[];

There is no need for DFA in this situation.

Thanks
--
Gustavo
Kalle Valo Dec. 5, 2023, 8 p.m. UTC | #5
"Gustavo A. R. Silva" <gustavo@embeddedor.com> writes:

> On 12/5/23 09:29, Kalle Valo wrote:
>> Jeff Johnson <quic_jjohnson@quicinc.com> writes:
>> 
>>> On 11/27/2023 8:23 AM, Gustavo A. R. Silva wrote:
>>>
>>>>
>>>>
>>>> On 11/27/23 10:14, Jeff Johnson wrote:
>>>>> Transform the zero-length array in ath11k_htc_record into a proper
>>>>> flexible array via the DECLARE_FLEX_ARRAY() macro. This helps with
>>>>> ongoing efforts to globally enable -Warray-bounds.
>>>>>
>>>>> Signed-off-by: Jeff Johnson <quic_jjohnson@quicinc.com>
>>>>> ---
>>>>>    drivers/net/wireless/ath/ath11k/htc.h | 2 +-
>>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/net/wireless/ath/ath11k/htc.h b/drivers/net/wireless/ath/ath11k/htc.h
>>>>> index 84971cc9251c..e0434b29df70 100644
>>>>> --- a/drivers/net/wireless/ath/ath11k/htc.h
>>>>> +++ b/drivers/net/wireless/ath/ath11k/htc.h
>>>>> @@ -151,7 +151,7 @@ struct ath11k_htc_credit_report {
>>>>>    struct ath11k_htc_record {
>>>>>    	struct ath11k_htc_record_hdr hdr;
>>>>>    	union {
>>>>> -		struct ath11k_htc_credit_report credit_report[0];
>>>>> +		DECLARE_FLEX_ARRAY(struct ath11k_htc_credit_report, credit_report);
>>>>>    	};
>>>>
>>>> Why not removing the `union` and just do a direct transformation [0] -> [ ] ?
>>>
>>> No reason other than staying consistent with ath10k.
>>> Will see if Kalle has an opinion on this.
>> Yeah, I don't see the need for the union and I removed it in the
>> pending
>> branch:
>> https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=a2faeea1fe0635563187e7821a6d0baf7b40f2c6
>> Does it look ok?
>> 
>
> Nope.
>
> A direct transformation is just fine:
>
> -	union {
> -		struct ath11k_htc_credit_report credit_report[0];
> -	};
> +	struct ath11k_htc_credit_report credit_report[];
>
> There is no need for DFA in this situation.

Sorry, I read your comments too hastily. Jeff, as I'm offline tomorrow
would you mind submitting v2?
diff mbox series

Patch

diff --git a/drivers/net/wireless/ath/ath11k/htc.h b/drivers/net/wireless/ath/ath11k/htc.h
index 84971cc9251c..e0434b29df70 100644
--- a/drivers/net/wireless/ath/ath11k/htc.h
+++ b/drivers/net/wireless/ath/ath11k/htc.h
@@ -151,7 +151,7 @@  struct ath11k_htc_credit_report {
 struct ath11k_htc_record {
 	struct ath11k_htc_record_hdr hdr;
 	union {
-		struct ath11k_htc_credit_report credit_report[0];
+		DECLARE_FLEX_ARRAY(struct ath11k_htc_credit_report, credit_report);
 	};
 } __packed __aligned(4);