Re: Rule 10.1 violations in perfc_incra and PERFCOUNTER_ARRAY

[Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>]

On 10/10/2023 03:56, Stefano Stabellini wrote:
> On Fri, 6 Oct 2023, Nicola Vetrini wrote:
> > Given the following macros in <xen/perfc.h> and
> >
> > #define perfc_incra(x,y)                                               
> >  \
> >     ( (y) <= PERFC_LAST_ ## x - PERFC_ ## x ?                          
> >  \
> >          ++this_cpu(perfcounters)[PERFC_ ## x + (y)] : 0 )
> >
> > and the following violation
> >
> > xen/arch/arm/traps.c:1407.5-1407.32:
> >   reference to enum constant `PERFC_hypercalls' has essential type 
> > `enum
> > perfcounter'
> >   and standard type `int'
> >  <preprocessed xen/arch/arm/traps.c>:11606.29-11606.44: preprocessed 
> > tokens
> >  <scratch space>:137.1-137.16: expanded from macro `PERFC_'
> >  xen/include/xen/perfc.h:69.35-69.45: expanded from macro 
> > `perfc_incra'
> > xen/arch/arm/traps.c:1407.5-1407.32:
> >   `+' addition operator expects a number or a character
> >  <preprocessed xen/arch/arm/traps.c>:11606.46: preprocessed tokens
> >  xen/include/xen/perfc.h:69.47: expanded from macro `perfc_incra'
> >
> > the difference between enumerated values is forbidden by the Rule. In 
> > the
> > coding standard's
> > interpretation, named enums are unordered list of symbols, which can 
> > only be
> > compared for
> > equality.
> > There are a few possible paths forward:
> >
> > 1. use means different from named enums to generate these constants 
> > (such as
> > #define-s or
> >    constants integers);
> This is a viable option
>
>
> > 2. explicitly deviate subtraction of enums, therefore defining an 
> > explicit
> > ordering on
> >    enumerated values;
> I would prefer to avoid a project-wide deviation, because I think in
> general it is a good rule to have. If we go with a deviation, I think 
> it
> would be better to deviate PERFCOUNTER or perfc.h specifically. This is
> a file that changes very rarely. We could make the case that this is
> safe with GCC (most probably is) and this case was reviewed carefully
> and doesn't change often (3 changes in the last 10 yeas).
>
>
> > 3. use an unnamed enum, effectively considering the enumerated values 
> > as plain
> > integers.
> >    This does not violate the Rule.
> What do you mean by unname enum?
e.g.

enum {
#include <xen/perfc_defn.h>
        NUM_PERFCOUNTERS
};

instead of

enum perfcounter {
#include <xen/perfc_defn.h>
        NUM_PERFCOUNTERS
};

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