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

[Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>]

On 2025-07-14 15:49, Jan Beulich wrote:
> On 14.07.2025 15:26, 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.
> > Use a ternary operator to replace it with a numeric value.
> >
> > 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 v2:
> > - improve the wording
> > Link to v2: 
> > https://patchew.org/Xen/41538b6b19811eb74c183051d3e7a4fd216404e6.1752232902.git.dmytro._5Fprokopchuk1@xxxxxxxx/
> > Link to the deviation of an unary minus: 
> > https://patchew.org/Xen/7e6263a15c71aafc41fe72cecd1f15c3ce8846f2.1752492180.git.dmytro._5Fprokopchuk1@xxxxxxxx/
> > Jan, regarding that:
> > If an expression is needed here, I'd suggest to use !!, as we have in
> > (luckily decreasing) number of places elsewhere. Personally I don't
> > understand though why a boolean cannot be used as an array index.
> >
> > The '!!' isn't a solution here, we'll have other violation:
> > `!' logical negation operator has essential type boolean and standard 
> > type `int'
> > (https://saas.eclairit.com:3787/fs/var/local/eclair/xen-project.ecdf/xen-project/people/dimaprkp4k/xen/ECLAIR_normal/deviate_10.1_rule/ARM64/10674114852/PROJECT.ecd;/by_service/MC3A2.R10.1.html#{%22select%22:true,%22selection%22:{%22hiddenAreaKinds%22:[],%22hiddenSubareaKinds%22:[],%22show%22:false,%22selector%22:{%22enabled%22:true,%22negated%22:true,%22kind%22:0,%22domain%22:%22kind%22,%22inputs%22:[{%22enabled%22:true,%22text%22:%22violation%22}]}}})
> And that doesn't fall under any other of the deviations we already 
> have?
> __isleap() is used in another boolean context after all, and apparently
> there's no issue there.
Hi Jan,

I double-checked: there is no specific deviation for using a boolean as 
an array subscript.
This is the only problematic use of a macro that returns an essentially 
boolean expr used as an operand to an operator that expects an integer, 
which is the reason of the violation:
xen/common/time.c:#define __isleap(year) \
xen/common/time.c:    while ( days >= (rem = __isleap(y) ? 366 : 365) )
xen/common/time.c:        days += __isleap(y) ? 366 : 365;
xen/common/time.c:    ip = (const unsigned short int 
*)__mon_lengths[__isleap(y)];
Thanks,
 Nicola

> > Well, in our case boolean can be used as an array index.
> > But index value is limited: 0 or 1.
> > I guess MISRA wants to predict such errors related to index 
> > limitations.
> > And I think fixing the code is easier here, instead of writing a 
> > deviation.
> It may be easier indeed, but ...
>
> > --- a/xen/common/time.c
> > +++ b/xen/common/time.c
> > @@ -84,7 +84,7 @@ struct tm gmtime(unsigned long t)
> >      }
> >      tbuf.tm_year = y - 1900;
> >      tbuf.tm_yday = days;
> > -    ip = (const unsigned short int *)__mon_lengths[__isleap(y)];
> > +    ip = (const unsigned short int *)__mon_lengths[__isleap(y) ? 1 : 
> > 0];
> ... especially as long as it's un-annotated, I'd be very likely to 
> submit
> a patch to undo this again, should I ever run across this after having
> forgotten about the change here. At least to me, _this_ is the 
> confusing
> way to write things.
>
> Once you add a comment though, you can as well leave the code unchanged
> and use a SAF comment.
>
> Jan
--
Nicola Vetrini, B.Sc.
Software Engineer
BUGSENG (https://bugseng.com)
LinkedIn: https://www.linkedin.com/in/nicola-vetrini-a42471253