Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 6 Sep 2022 15:20:51 -0700
From:   Brian Norris <briannorris@chromium.org>
To:     Johannes Berg <johannes@sipsolutions.net>
Cc:     linux-wireless@vger.kernel.org,
        Johannes Berg <johannes.berg@intel.com>
Subject: Re: [PATCH 07/12] wifi: mwifiex: fix array of flexible structures
 warnings
Message-ID: <YxfHwxuwEP1rQoAU@google.com>
References: <20220904212910.8169e8c9090c.I0357e80cc86be2d4ac6205d1f53568444dcf7c9b@changeid>
 <20220904212910.2c885310ebee.If7177ea588b56c405eee6e6df595e9efccdfb99a@changeid>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220904212910.2c885310ebee.If7177ea588b56c405eee6e6df595e9efccdfb99a@changeid>
Precedence: bulk

Hi,

On Sun, Sep 04, 2022 at 09:29:07PM +0200, Johannes Berg wrote:
> From: Johannes Berg <johannes.berg@intel.com>
> 
> There are two, just change them to have a "u8 data[]" type
> member, and add casts where needed. No binary changes.

Hmm, what exact warning are you looking at? This one?
https://clang.llvm.org/docs/DiagnosticsReference.html#wflexible-array-extensions

It's a little hard to suggest alternatives (or understand why this is
the only/best way) if I don't know what the alleged bug/warning is.

> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> ---
>  drivers/net/wireless/marvell/mwifiex/fw.h      | 4 ++--
>  drivers/net/wireless/marvell/mwifiex/sta_cmd.c | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/wireless/marvell/mwifiex/fw.h b/drivers/net/wireless/marvell/mwifiex/fw.h
> index 26a48d8f49be..b4f945a549f7 100644
> --- a/drivers/net/wireless/marvell/mwifiex/fw.h
> +++ b/drivers/net/wireless/marvell/mwifiex/fw.h
> @@ -2104,7 +2104,7 @@ struct mwifiex_fw_mef_entry {
>  struct host_cmd_ds_mef_cfg {
>  	__le32 criteria;
>  	__le16 num_entries;
> -	struct mwifiex_fw_mef_entry mef_entry[];
> +	u8 mef_entry_data[];

Do you actually need this part of the change? The 'mef_entry' (or
'mef_entry_data') field is not actually used anywhere now, but I can't
tell what kind of warning is involved.

But also see the next comment:

>  } __packed;
>  
>  #define CONNECTION_TYPE_INFRA   0
> @@ -2254,7 +2254,7 @@ struct coalesce_receive_filt_rule {
>  struct host_cmd_ds_coalesce_cfg {
>  	__le16 action;
>  	__le16 num_of_rules;
> -	struct coalesce_receive_filt_rule rule[];
> +	u8 rule_data[];

These kinds of changes seem to be losing some valuable information. At a
minimum, it would be nice to leave a comment that points at the intended
struct; but ideally, we'd be able to still get the type safety from
actually using the struct, instead of relying on casts and/or u8/void.

But I don't know if that's possible, as I'm not familiar with the
compiler warning involved.

Brian