Re: [PATCH] brcmfmac: fwsignal: Use struct_size() to simplify brcmf_fws_rxreorder()

[Arend van Spriel <arend.vanspriel@broadcom.com>]

[-- Attachment #1: Type: text/plain, Size: 2131 bytes --]

On 7/14/2024 12:10 PM, Christophe JAILLET wrote:
> In the "struct brcmf_ampdu_rx_reorder", change the 'pktslots' field into
> flexible array.
> 
> It saves the size of a pointer when the memory is allocated and avoids
> an indirection when the array is used.
> It also removes the usage of a pointer arithmetic and saves a few lines of
> code.
> 
> Finally, struct_size() can be used. It is not a must have here, because
> it is easy to see that buf_size can not overflow, but still, it is a good
> practice.

Looks good to me with a minor nit (see below)...

Acked-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
> Compile tested only
> ---
>   drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h   | 4 ++--
>   .../net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.c   | 8 ++------
>   2 files changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
> index ea76b8d33401..6ea2b677f047 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
> @@ -48,20 +48,20 @@
>   /**
>    * struct brcmf_ampdu_rx_reorder - AMPDU receive reorder info
>    *
> - * @pktslots: dynamic allocated array for ordering AMPDU packets.
>    * @flow_id: AMPDU flow identifier.
>    * @cur_idx: last AMPDU index from firmware.
>    * @exp_idx: expected next AMPDU index.
>    * @max_idx: maximum amount of packets per AMPDU.
>    * @pend_pkts: number of packets currently in @pktslots.
> + * @pktslots: dynamic allocated array for ordering AMPDU packets.

pktslots is not dynamic allocated, but just part of the structure. That 
was already so before this patch through. Just drop the "dynamic 
allocated" part.

>    */
>   struct brcmf_ampdu_rx_reorder {
> -	struct sk_buff **pktslots;
>   	u8 flow_id;
>   	u8 cur_idx;
>   	u8 exp_idx;
>   	u8 max_idx;
>   	u8 pend_pkts;
> +	struct sk_buff *pktslots[];
>   };
>   
>   /* Forward decls for struct brcmf_pub (see below) */

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4219 bytes --]