On Fri, Aug 25, 2023 at 01:37:09PM -0700, Kees Cook wrote: > On Sun, Aug 06, 2023 at 10:05:54AM -0700, James Seo wrote: >> Note that iounit_pg8 occurs in the middle of the per-adapter >> struct, not at the end. The per-adapter struct is extensively > > This is especially bad/weird. Flex arrays aren't supposed to live there, > so I think it'd be best to avoid this conversion (see below). > >> used throughout mpt3sas even if its iounit_pg8 member isn't, >> resulting in an especially large amount of noise when comparing >> binary changes attributable to this commit. > > Since the size reduction makes it hard to validate, how about just > leaving it alone? Since nothing is using Sensor[], you could just make > it a single instance: > > - MPI2_IOUNIT8_SENSOR > - Sensor[MPI2_IOUNITPAGE8_SENSOR_ENTRIES];/*0x10 */ > + MPI2_IOUNIT8_SENSOR Sensor; /*0x10 */ > > > or leave it as-is (i.e. drop this patch). > I'd prefer not to paper it over by just up and pretending it's not a flex array at all, but leaving things as-is feels like a waste, and I understand the need to be conservative with storage drivers. How do you feel about removing the struct containing the flex array from the middle of the per-adapter struct, as per patch 8 in this series? Moving that patch before this one in the ordering would cleanly fix the misplaced flex array, but I imagine you'd be especially keen on seeing Broadcom's approval for that one. In any case, I'm fine with turning this into a single instance if it comes down to it.
