On 28/10/24 17:21, Jakub Kicinski wrote:
On Mon, 21 Oct 2024 13:02:27 -0600 Gustavo A. R. Silva wrote:
Fix 3338 of the following -Wflex-array-member-not-at-end warnings:
include/linux/ethtool.h:214:38: warning: structure containing a flexible array
member is not at the end of another structure [-Wflex-array-member-not-at-end]
I don't see any change in the number of warnings with W=1:
gcc (GCC) 14.2.1 20240912 (Red Hat 14.2.1-3)
Is it only enabled with W=2?
-Wfamnae is not currently part of any upstream build. We are working
to have it enabled. So, these warnings are the ones I see in my local
build with the following patch applied:
diff --git a/Makefile b/Makefile
index d4a41c44e0fc..08d18b5d01f5 100644
--- a/Makefile
+++ b/Makefile
@@ -1002,6 +1002,9 @@ NOSTDINC_FLAGS += -nostdinc
# perform bounds checking.
KBUILD_CFLAGS += $(call cc-option, -fstrict-flex-arrays=3)
+# Avoid flexible-array members not at the end of composite structure.
+KBUILD_CFLAGS += $(call cc-option, -Wflex-array-member-not-at-end)
+
#Currently, disable -Wstringop-overflow for GCC 11, globally.
KBUILD_CFLAGS-$(CONFIG_CC_NO_STRINGOP_OVERFLOW) += $(call cc-option,
-Wno-stringop-overflow)
KBUILD_CFLAGS-$(CONFIG_CC_STRINGOP_OVERFLOW) += $(call cc-option,
-Wstringop-overflow)
Additionally, update the type of some variables in various functions
that don't access the flexible-array member, changing them to the
newly created `struct ethtool_link_settings_hdr`.
Why? Please avoid unnecessary code changes.
This is actually necessary. As the type of the conflicting middle members
changed, those instances that expect the type to be `struct
ethtool_link_settings`
should be adjusted to the new type. Another option is to leave the type
unchanged and instead use container_of. See below.
So, instead of this:
- struct ethtool_link_settings *base = &link_ksettings->base;
+ struct ethtool_link_settings_hdr *base = &link_ksettings->base;
we would do something like this:
- struct ethtool_link_settings *base = &link_ksettings->base;
+ struct ethtool_link_settings *base = container_of(&link_ksettings->base,
+ struct struct
ethtool_link_settings, hdr);
I think that in this case, we could avoid using `container_of()`, but
if you prefer that, I can update the patch.
include/linux/ethtool.h | 2 +-
This is probably where most of the warnings come from.
Please split the changes to this header file as a separate patch
for ease of review / validation.
Sure thing!
Thanks
--
Gustavo
