Jeff Johnson <quic_jjohnson@xxxxxxxxxxx> writes:
> On 8/9/2024 9:20 AM, Gustavo A. R. Silva wrote:
>
>> -Wflex-array-member-not-at-end was introduced in GCC-14, and we are
>> getting ready to enable it, globally.
>>
>> Move the conflicting declaration to the end of the structure. Notice
>> that `struct ieee80211_chanctx_conf` is a flexible structure --a
>> structure that contains a flexible-array member.
>>
>> Also, remove a couple of unused structures.
>>
>> Fix the following warnings:
>> drivers/net/wireless/ath/ath11k/core.h:409:39: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
>> drivers/net/wireless/ath/ath11k/dp.h:1309:24: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
>> drivers/net/wireless/ath/ath11k/dp.h:1368:24: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
>>
>> Signed-off-by: Gustavo A. R. Silva <gustavoars@xxxxxxxxxx>
>> ---
>> drivers/net/wireless/ath/ath11k/core.h | 4 +++-
>> drivers/net/wireless/ath/ath11k/dp.h | 23 -----------------------
>> 2 files changed, 3 insertions(+), 24 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath11k/core.h b/drivers/net/wireless/ath/ath11k/core.h
>> index df24f0e409af..e283415dccf3 100644
>> --- a/drivers/net/wireless/ath/ath11k/core.h
>> +++ b/drivers/net/wireless/ath/ath11k/core.h
>> @@ -406,11 +406,13 @@ struct ath11k_vif {
>> bool wpaie_present;
>> bool bcca_zero_sent;
>> bool do_not_send_tmpl;
>> - struct ieee80211_chanctx_conf chanctx;
>> struct ath11k_arp_ns_offload arp_ns_offload;
>> struct ath11k_rekey_data rekey_data;
>>
>> struct ath11k_reg_tpc_power_info reg_tpc_info;
>> +
>> + /* Must be last - ends in a flexible-array member. */
>> + struct ieee80211_chanctx_conf chanctx;
>
> there is something illogical about this since the vif is allocated using
> sizeof() and hence there will never be memory allocated for the flexible
> array, and it is assigned using either struct assignment or memcpy using the
> struct size which (fortunately) would not transfer the flexible array contents:
> arvif->chanctx = *ctx;
>
> memcpy(&arvif->chanctx, ctx, sizeof(*ctx));
>
> since ath11k doesn't actually use the drv_priv[] I guess this change is OK, it
> is just strange to me.
>
> also makes me wonder why ath11k keeps a copy of the chanctx instead of just
> getting it from the underlying ieee80211_link_data. but that is outside the
> scope of this discussion.
Yeah, this doesn't look right. I don't think a driver should be copying
struct ieee80211_chanctx_conf like that. I think I'll add a comment
about this to the code:
/* FIXME: Driver should not copy struct ieee80211_chanctx_conf,
* especially because it has a flexible array. Find a better way.
*/
Thoughts?
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
https://docs.kernel.org/process/submitting-patches.html
