Re: Rule 10.1 violations in perfc_incra and PERFCOUNTER_ARRAY

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

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?