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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 28.07.2025 15:09, Dmytro Prokopchuk1 wrote:
> 
> 
> 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?

I expect so, as a temporary measure. In the longer run I would hope Eclair
can be adjusted to accept such cases without complaint.

Jan