Re: [PATCH 2/2][next] net: ethtool: Avoid thousands of -Wflex-array-member-not-at-end warnings

From: Gustavo A. R. Silva
Date: Mon Oct 28 2024 - 22:37:34 EST




On 28/10/24 18:32, Jakub Kicinski wrote:
On Mon, 28 Oct 2024 17:32:53 -0600 Gustavo A. R. Silva wrote:
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.

Ah, that makes sense. So they need to be included int the newly split
patch. Please rephrase the commit message a bit, the current paragraph
reads as if this was a code cleanup.

After double-checking, it turns out that the patch ends up being basically
the same. The only change that would be split in a separate patch would be
the following.

diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
index 5cc131cdb1bc..7da94e26ced6 100644
--- a/net/ethtool/ioctl.c
+++ b/net/ethtool/ioctl.c
@@ -425,7 +425,7 @@ convert_link_ksettings_to_legacy_settings(

/* layout of the struct passed from/to userland */
struct ethtool_link_usettings {
- struct ethtool_link_settings base;
+ struct ethtool_link_settings_hdr base;
struct {
__u32 supported[__ETHTOOL_LINK_MODE_MASK_NU32];
__u32 advertising[__ETHTOOL_LINK_MODE_MASK_NU32];

The rest will essentially remain the same as the change in
include/linux/ethtool.h triggers a cascade of changes across
the rest of the files in this patch.

So, you tell me if you still want me to split this patch. In any case
I'll update the changelog text.

Thanks
--
Gustavo