Re: [XEN PATCH][for-next v2 6/8] x86/mce: Move MC_NCLASSES into the enum mctelem_class

[Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>]

On 17/10/2023 11:54, Jan Beulich wrote:
> On 17.10.2023 11:43, Nicola Vetrini wrote:
> > On 17/10/2023 10:26, Jan Beulich wrote:
> > > On 17.10.2023 10:12, Nicola Vetrini wrote:
> > > > On 17/10/2023 09:02, Jan Beulich wrote:
> > > > > On 16.10.2023 18:05, Nicola Vetrini wrote:
> > > > > > On 16/10/2023 17:45, Jan Beulich wrote:
> > > > > > > On 12.10.2023 17:28, Nicola Vetrini wrote:
> > > > > > > > The definition of MC_NCLASSES contained a violation of MISRA
> > > > > > > > C:2012
> > > > > > > > Rule 10.1, therefore by moving it as an enumeration constant
> > > > > > > > resolves
> > > > > > > > the
> > > > > > > > violation and makes it more resilient to possible additions to
> > > > > > > > that
> > > > > > > > enum.
> > > > > > > And using an enumerator as array dimension specifier is okay for
> > > > > > > Misra?
> > > > > > > That would be odd when elsewhere named enums are treated 
> > > > > > > specially.
> > > > > > Yes, the array subscript operator is one of the few places where 
> > > > > > an
> > > > > > enum
> > > > > > can be used as
> > > > > > an operand (also because negative values wouldn't compile), as
> > > > > > opposed
> > > > > > to mixing them
> > > > > > with ordinary integers.
> > > > > When saying "odd" I didn't even think of negative values. May I
> > > > > therefore
> > > > > ask for the reasoning of why this specific case is deemed 
> > > > > non-risky?
> > > > > To
> > > > > me there looks to be a fair risk of creating undersized arrays,
> > > > > leading
> > > > > to out-of-bounds accesses.
> > > > Two reasons: MC_NCLASSES is the last value of the enum, and a 
> > > > pattern
> > > > I've spot in various
> > > > other places in Xen, so I assumed it was a fairly common pattern for
> > > > the
> > > > community.
> > > > The other reason is that since the value of an enum constant can be
> > > > derived statically, there
> > > > is little risk of it being the wrong value, because no arithmetic is
> > > > done on it in its use
> > > > as an array's size (and besides, the enum changed the last time ~15
> > > > years ago, so I think
> > > > it's unlikely to change much in the near future).
> > > You focus on the specific instance, yet my question was on the 
> > > general
> > > principle.
> > A couple of reasons why this is allowed:
> > - associating values to set of symbols is typical and makes sense in
> > some contexts
> > - out-of-bounds operations on arrays are dealt with by a host of other
> > guidelines
> >    (Series 18, mainly)
> > - this rule is about which kinds of operands makes sense to use with
> > certain operators.
> >    It was deemed unlikely by MISRA that risky behaviour may arise by
> > using symbolic indices
> >    as subscripts, given the rest of the other guidelines and the
> > unspecified and undefined
> >    associated with Rule 10.1. It's not impossible that problems will
> > arise, but far less
> >    likely than using enums with no restrictions at all (such as those
> > caused by an enum of
> >    and implementation-defined type used in an arithmetic operation, 
> > that
> > could give
> >    unexpected results).
> Now you appear to focus on uses of arrays, not their definition. Yet 
> even
> there I wonder: array[idx] is nothing else than *(array + idx). Adding
> integer types and enums is disallowed. Nothing is said about arithmetic
> on pointers throughout the description of the type model and its rules.
> So despite the restriction on integer types, adding enums to pointers 
> is
> permitted?
>
> Jan
It's not disallowed just to add enums to integers, but the operation of 
addition on
(named) enums. The MISRA C coding standard deemed arr[IDX] an 
appropriate (not to be
interpreted as safe, though what is unsafe is inappropriate as a 
consequence of the
essential type model) use of enums.

--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)