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

["Gustavo A. R. Silva" <gustavo@embeddedor.com>]

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