Re: [PATCH][next] wifi: ath11k: Avoid -Wflex-array-member-not-at-end warnings

Jeff Johnson [off-list ref] writes:

On 8/9/2024 9:20 AM, Gustavo A. R. Silva wrote:

quoted

-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@kernel.org>
---
 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