Re: [PULL 53/60] hw/cxl: Standardize all references on CXL r3.1 and minor updates

[Peter Maydell]

On Fri, 8 Mar 2024 at 14:34, Jonathan Cameron
<Jonathan.Cameron@huawei.com> wrote:
>
> On Fri, 8 Mar 2024 13:47:47 +0000
> Peter Maydell <peter.maydell@linaro.org> 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