On Fri, 8 Mar 2024 at 14:34, Jonathan Cameron <[email protected]> wrote: > > On Fri, 8 Mar 2024 13:47:47 +0000 > Peter Maydell <[email protected]> wrote: > > Is there a way we could write this that would catch this error? > > I'm thinking maybe something like > > > > #define CXL_CREATE_DVSEC(CXL, DEVTYPE, TYPE, DATA) do { \ > > assert(sizeof(*DATA) == TYPE##_LENGTH); \ > > cxl_component_create_dvsec(CXL, DEVTYPE, TYPE##_LENGTH, \ > > TYPE, TYPE##_REVID, (uint8_t*)DATA); \ > > } while (0) > > We should be able to use the length definitions in the original assert. > I'm not sure why that wasn't done before. I think there were some cases > where we supported multiple versions and so the length can be shorter > than the structure defintion but that doesn't matter on this one. > > So I think minimal fix is u16 of padding and update the assert. > Can circle back to tidy up the multiple places the value is defined. > Any mismatch in which the wrong length define is used should be easy > enough to spot so not sure we need the macro you suggest.
Well, I mean, you didn't in fact spot the mismatch between the struct type you were passing and the length value you were using. That's why I think it would be helpful to assert() that the size of the struct really does match the length value you're passing in. At the moment the code completely throws away the type information the compiler has by casting the pointer to the struct to a uint8_t*. thanks -- PMM
