Re: [PATCH v1 1/2] xen/page_alloc: address violation of Rule 14.3

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 28 Apr 2025, Jan Beulich wrote:
> On 26.04.2025 02:00, victorm.lira@xxxxxxx wrote:
> > From: Federico Serafini <federico.serafini@xxxxxxxxxxx>
> > 
> > MISRA C Rule 14.3 states that "Controlling expressions shall not be
> > invariant".
> > 
> > Add a SAF comment to deviate the rule for build configurations without
> > CONFIG_LLC_COLORING enabled.
> 
> I was surprised by this supposedly being the only violation. And indeed it
> wasn't very hard to find more. For example, we have a number of
> "while ( num_online_cpus() > 1 && ... )", which become compile-time
> constant (false) when NR_CPUS=1.

Uhm, I did run a special scan for this and I can confirm no other
violations are detected.


> > --- a/xen/common/page_alloc.c
> > +++ b/xen/common/page_alloc.c
> > @@ -2038,6 +2038,7 @@ static struct page_info 
> > *alloc_color_heap_page(unsigned int memflags,
> > 
> >      spin_lock(&heap_lock);
> > 
> > +    /* SAF-14-safe MISRA C R14.3 condition always false without 
> > LLC_COLORING */
> >      for ( i = 0; i < domain_num_llc_colors(d); i++ )
> >      {
> >          unsigned long free = free_colored_pages[domain_llc_color(d, i)];
> 
> Hmm, this way the deviation applies even when LLC_COLORING=y.

Yes but in the LLC_COLORING=y case it is harmless. Do you have something
else in mind?


> As to the comment wording - looks like we're pretty inconsistent with that
> right now. I, for one, don't think the Misra rule needs (re)stating there;
> the SAF index points at all the data that's needed if one cares about the
> specifics of the deviation.

Do you prefer:

/* SAF-14-safe */

?