Re: [PATCH] scsi: qla2xxx: Remove problematic BUILD_BUG_ON() assertion

[Finn Thain]

On Fri, 6 Mar 2026, Tony Battersby wrote:

> On 3/5/26 18:01, Finn Thain wrote:
> ...
> > I don't know of a good way to encode an invariant like "the last 
> > member of struct qla_tgt_sess_op is named atio" such that it might be 
> > statically checked. But perhaps there is a good way to do that (?)
> 
> It might work better to add a flex array:
> 
> struct qla_tgt_sess_op {
> 	...
> 
> 	struct atio_from_isp atio;
> 	/*
> 	atio.u.isp24.fcp_cmnd.add_cdb may extend past end of atio;
> 	DO NOT DELETE; DO NOT ADD ANYTHING ELSE HERE.
> 	*/
> 	uint8_t atio_isp24_fcp_cmnd_add_cdb[];
> };
> 
> /* atio_isp24_fcp_cmnd_add_cdb must come immediately after atio */
> BUILD_BUG_ON(offsetof(struct qla_tgt_sess_op, atio) +
>              sizeof(struct atio_from_isp) !=
>              offsetof(struct qla_tgt_sess_op, atio_isp24_fcp_cmnd_add_cdb));
> 

I think that makes sense because the flex array member reflects what the 
algorithm actually does (whereas artificial struct padding would not).

And it works better than the present code, because the compiler prohibits 
any new member at the end of the struct.

It is more complex than the patch I sent but maintainers may still prefer 
it, so I will put it into a formal patch submission.

BTW, I thought it would make more sense to add a flex array in struct 
isp24 (in struct atio_from_isp) but it doesn't work because the compiler 
doesn't prohibit aggregation:

struct s1 {
        int i;
        int a[];
};

struct s2 {
        struct s1 s;
        int x;        /* this is not prohibited */
};