Re: [PATCH] xen/public: fix flexible array definitions

[Juergen Gross <jgross@xxxxxxxx>]

On 09.08.23 11:42, Juergen Gross wrote:
> On 26.07.23 07:52, Jan Beulich wrote:
> > On 25.07.2023 18:59, Andrew Cooper wrote:
> > > On 25/07/2023 5:16 pm, Jan Beulich wrote:
> > > > On 25.07.2023 15:55, Juergen Gross wrote:
> > > > > Flexible arrays in public headers can be problematic with some
> > > > > compilers.
> > > > >
> > > > > Replace them with arr[XEN_FLEX_ARRAY_DIM] in order to avoid compilation
> > > > > errors.
> > > > >
> > > > > This includes arrays defined as "arr[1]", as seen with a recent Linux
> > > > > kernel [1].
> > > > >
> > > > > [1]: https://bugzilla.kernel.org/show_bug.cgi?id=217693
> > > > >
> > > > > Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
> > > > I think we need to be careful here: What if someone somewhere applies
> > > > sizeof() to any of the types you alter?
> > > Then the code was most likely wrong already.
> > That's possible to judge only when seeing the code in question.
> >
> > > >   The resulting value would
> > > > change with the changes you propose, which we cannot allow to happen
> > > > in a stable interface. Therefore imo it can only be an opt-in feature
> > > > to have these arrays no longer be one-element ones.
> > > I don't consider this an issue.
> > >
> > > If people take an update to the headers and their code stops compiling,
> > > then of course they fix the compilation issue.  That's normal.
> > The code may continue to compile fine, and even appear to work initially.
> >
> > > It's unreasonable to take opt-in features to a set of headers intended
> > > to be vendored in the first place, to work around a corner case that's
> > > likely buggy already.
> > The original intention clearly was to allow use of these headers as is.
> > Anyway, I've voiced my view, yet if there are enough people agreeing
> > with you, then so be it.
> Any further thoughts?
>
> I have checked the code in the Linux kernel meanwhile. There should be no
> fallout resulting from this change, but I think there are some user mode
> backends outside of qemu which are probably using affected structs.
I've received another mail regarding the report [1] above. I think we should
_really_ come to a conclusion.

I'm still in favor of applying my suggested patch.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature