Re: [PATCH] misra: deviate explicit cast for Rule 11.1

[Dmytro Prokopchuk1 <dmytro_prokopchuk1@xxxxxxxx>]

On 7/28/25 12:56, Jan Beulich wrote:
> On 27.07.2025 22:27, Dmytro Prokopchuk1 wrote:
>> Explicitly cast 'halt_this_cpu' when passing it
>> to 'smp_call_function' to match the required
>> function pointer type '(void (*)(void *info))'.
>>
>> Document and justify a MISRA C R11.1 deviation
>> (explicit cast).
>>
>> Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@xxxxxxxx>
> 
> All you talk about is the rule that you violate by adding a cast. But what is
> the problem you're actually trying to resolve by adding a cast?
> 
>> --- a/xen/arch/arm/shutdown.c
>> +++ b/xen/arch/arm/shutdown.c
>> @@ -25,7 +25,8 @@ void machine_halt(void)
>>       watchdog_disable();
>>       console_start_sync();
>>       local_irq_enable();
>> -    smp_call_function(halt_this_cpu, NULL, 0);
>> +    /* SAF-15-safe */
>> +    smp_call_function((void (*)(void *))halt_this_cpu, NULL, 0);
> 
> Now this is the kind of cast that is very dangerous. The function's signature
> changing will go entirely unnoticed (by the compiler) with such a cast in 
> place.
> 
> If Misra / Eclair are unhappy about such an extra (benign here) attribute, I'd
> be interested to know what their suggestion is to deal with the situation
> without making the code worse (as in: more risky). I first thought about 
> having
> a new helper function that then simply chains to halt_this_cpu(), yet that
> would result in a function which can't return, but has no noreturn attribute.
> 
> Jan

Yes, Misra doesn't like cast.

Initially Misra reported about non-compliant implicit cast due to 
'noreturn' attribute:
smp_call_function(halt_this_cpu, NULL, 0);

I thought that in this case explicit cast is better, telling compiler 
exact type.
But, Misra reported about non-compliant c-style (explicit) cast.
So, I decided to deviate explicit cast.

I tried to write wrapper function to resolve this.
Example:
static void halt_this_cpu_2(void *arg)
{
     halt_this_cpu(arg);
}
void machine_halt(void)
{
     ...
     smp_call_function(halt_this_cpu_2, NULL, 0);
     ...

Unfortunately new R2.1 violation was observed.
"function definition `halt_this_cpu_2(void*)' (unit 
`xen/arch/arm/shutdown.c' with target `xen/arch/arm/shutdown.o') will 
never return"

Maybe it's better to have such violation....instead of R11.1 
"non-compliant cast"


I can remove cast and re-write deviation justification.
Are you OK with that, Jan?

Dmytro