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

[Gustavo A. R. Silva]

On 01/04/24 22:24, Jakub Kicinski wrote:
> On Thu, 28 Mar 2024 19:17:10 -0600 Gustavo A. R. Silva wrote:
> > --- a/drivers/net/ethernet/netronome/nfp/nfp_net_debugdump.c
> > +++ b/drivers/net/ethernet/netronome/nfp/nfp_net_debugdump.c
> > @@ -34,8 +34,11 @@ enum nfp_dumpspec_type {
> >   
> >   /* generic type plus length */
> >   struct nfp_dump_tl {
> > -	__be32 type;
> > -	__be32 length;	/* chunk length to follow, aligned to 8 bytes */
> > +	/* New members must be added within the struct_group() macro below. */
> > +	struct_group_tagged(nfp_dump_tl_hdr, hdr,
> > +		__be32 type;
> > +		__be32 length;	/* chunk length to follow, aligned to 8 bytes */
> > +	);
> >   	char data[];
> >   };
>
> I counted 9 references to nfp_dump_tl->data.
> Better to add:
>
> static void *nfp_dump_tl_data(struct nfp_dump_tl *spec)
> {
> 	return &spec[1];
> }

Unfortunately, that's out-of-bounds for the compiler, and well, basically
the reason why flex-array members were created in the first place.

I was looking into implementing two separate structs:

struct nfp_dump_tl_hdr {
        __be32 type;
        __be32 length;  /* chunk length to follow, aligned to 8 bytes */
};

struct nfp_dump_tl {
        __be32 type;
        __be32 length;  /* chunk length to follow, aligned to 8 bytes */
	char data[];
};

and at least for structs nfp_dumpspec_csr, nfp_dumpspec_rtsym, nfp_dump_csr, and
nfp_dump_rtsym it'd be a clean change (no need for container_of()), but not for
structs nfp_dumpspec_csr and nfp_dumpspec_rtsym because of some casts from
the flex struct:

nfp_add_tlv_size():
        case NFP_DUMPSPEC_TYPE_ME_CSR:
                spec_csr = (struct nfp_dumpspec_csr *)tl;
                if (!nfp_csr_spec_valid(spec_csr))
		...

        case NFP_DUMPSPEC_TYPE_INDIRECT_ME_CSR:
                spec_csr = (struct nfp_dumpspec_csr *)tl;
                if (!nfp_csr_spec_valid(spec_csr))
		...

	case NFP_DUMPSPEC_TYPE_RTSYM:
                spec_rtsym = (struct nfp_dumpspec_rtsym *)tl;
                err = nfp_dump_single_rtsym(pf, spec_rtsym, dump);

nfp_calc_rtsym_dump_sz():
        spec_rtsym = (struct nfp_dumpspec_rtsym *)spec;


At least for those two structs, it's probably more straightforward to use
struct_group_tagged() and container_of().

--
Gustavo