On 28/07/2025 10:56 am, 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.
I agree. This code is *far* safer in practice without the cast, than with it. > 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. I guess that Eclair cannot know what an arbitrary attribute does and whether it impacts the ABI, but it would be lovely if Eclair could be told "noreturn is a safe attribute to differ on"? ~Andrew
