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_prokopch...@epam.com>
>>
>> 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
