Re: [RFC 1/4] x86/ioemul: address MISRA C:2012 Rule 9.3

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 13 Nov 2023, Jan Beulich wrote:
> On 11.11.2023 02:23, Stefano Stabellini wrote:
> > On Mon, 6 Nov 2023, Nicola Vetrini wrote:
> >>>>> There's also this functionally equivalent alternative, with or without
> >>>>> the zeros, which
> >>>>> doesn't incur in the risk of mistakenly attempting to initialize the
> >>>>> same element twice,
> >>>>> while also giving an explicit cue to the reader that all elements are
> >>>>> truly zero-initialized.
> >>>>>
> >>>>>           .matches = {
> >>>>>               DMI_MATCH(DMI_BIOS_VENDOR, "HP"),
> >>>>>               DMI_MATCH(DMI_PRODUCT_NAME, "ProLiant DL5"),
> >>>>> +            {0}, {0}
> >>>>>           },
> >>>>
> >>>> Adding a dependency on the array actually having 4 elements (while iirc
> >>>> we have seen already that we could in principle go down to 3). A change
> >>>> of this number would then require touching all these sites, which is
> >>>> what we'd like to avoid.
> >>>
> >>> How often the array needs to change though? Looking at the git history
> >>> it doesn't seem the number of elements ever changed. So I think it is a
> >>> good tradeoff, and I would go with this type of fix (maybe also at the
> >>> other locations mechanically too although I haven't looked at them in
> >>> details).
> >>
> >> Hi, any updates on this? Considering the opinions expressed above, what 
> >> would
> >> be the path preferred by the community?
> > 
> > Hi Jan, to bring this discussion to a conclusion, I think we have these
> > options:
> > 
> > 1) fix these violations by adding {}, {}
> > 2) fix these violations by adding [0]=xxx,[1]=xxx
> > 3) deviate these violations by adding /* SAF-safe-xxx */
> > 4) remove the MISRA rule 9.3 from docs/misra/rules.rst
> > 
> > Let's make a decision. My preference is 1) as we only have ~50
> > violations.
> 
> Of these, to be honest, my preference would be 4. Just that that's
> undesirable for other reasons. But have we thought of alternatives, say
> a variadic macro that would supply the "missing" initializers? Imo such
> decisions shouldn't be rushed; there are enough other issues to take
> care of in the meantime. A sound solution is, I think, generally
> preferable to a quick one. (Whether my new suggestion is "sound" I of
> course can't tell, until it was tried out and the overall result /
> effects can be inspected.)

I don't like the idea of the variadic macro as we should attempt to make
things more obviously correct, rather than more obscure.

Thinking out of the box, what if we added a single {} E.g.:

        .ident = "HP ProLiant DL3xx",
        .matches = {
            DMI_MATCH(DMI_BIOS_VENDOR, "HP"),
            DMI_MATCH(DMI_PRODUCT_NAME, "ProLiant DL3"),
            {}
        },

It would accomplish the goal of highlighting that there are more members
of the array that gets initialized to zero. At the same time it wouldn't
require the introductino of [0] and [1] that as we have seen are error
prone and it wouldn't depend on the exact number of elements like adding
one {} per missing initialization. To be clear, I am suggesting adding a
single {} only.

Nicola, what do you think? Would it be OK for MISRA / ECLAIR?