Re: [XEN PATCH v4] misra: address violation of MISRA C Rule 10.1

[Jan Beulich <jbeulich@xxxxxxxx>]

On 14.07.2025 19:53, Dmytro Prokopchuk1 wrote:
> Rule 10.1: Operands shall not be of an
> inappropriate essential type
> 
> The following are non-compliant:
> - boolean used as a numeric value.
> 
> The result of the '__isleap' macro is a boolean.
> Suppress analyser tool finding.
> 
> The result of 'NOW() > timeout' is a boolean,
> which is compared to a numeric value. Fix this.
> Regression was introdiced by commit:
> be7f047e08 (xen/arm: smmuv3: Replace linux functions with xen functions.)
> 
> Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@xxxxxxxx>
> ---
> Changes since v3:
> - added a SAF comment instead of using ternary operator
> - removed pointless cast
> - updated commit message
> 
> CI: 
> https://saas.eclairit.com:3787/fs/var/local/eclair/xen-project.ecdf/xen-project/people/dimaprkp4k/xen/ECLAIR_normal/deviate_10.1_rule/ARM64/10678198209/PROJECT.ecd;/by_service.html#service&kind
> ---
>  docs/misra/safe.json                  | 8 ++++++++
>  xen/common/time.c                     | 3 ++-
>  xen/drivers/passthrough/arm/smmu-v3.c | 2 +-
>  3 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/docs/misra/safe.json b/docs/misra/safe.json
> index e3489dba8e..964f8344ce 100644
> --- a/docs/misra/safe.json
> +++ b/docs/misra/safe.json
> @@ -116,6 +116,14 @@
>          },
>          {
>              "id": "SAF-14-safe",
> +            "analyser": {
> +                "eclair": "MC3A2.R10.1"
> +            },
> +            "name": "Rule 10.1: use boolean as an array index",
> +            "text": "Using a boolean type as an array index is safe because 
> the array size equals exactly two."

Isn't this too strict? Use of a boolean would be fine as well for larger
arrays. In fact, we allow integers (without compile-time known bounds) to
be used as array indexes, too. Hence I see no reason to mention array
dimension here at all.

> --- a/xen/common/time.c
> +++ b/xen/common/time.c
> @@ -84,7 +84,8 @@ struct tm gmtime(unsigned long t)
>      }
>      tbuf.tm_year = y - 1900;
>      tbuf.tm_yday = days;
> -    ip = (const unsigned short int *)__mon_lengths[__isleap(y)];
> +    /* SAF-14-safe use boolean as an array index */
> +    ip = __mon_lengths[__isleap(y)];

Thanks for dropping the cast, yet you doing so also needs to be mentioned
in the description (making clear this is deliberate, and why).

Jan