On 2023-12-12 10:45, Jan Beulich wrote:
On 11.12.2023 11:30, Nicola Vetrini wrote:
Given that 'hwdom_shutdown' is a noreturn function, unreachable
breaks can be eliminated to resolve violations of Rule 2.1.
On the occasion, the type of its parameter is changed to uint8_t.
I don't particularly mind that extra change, but it's entirely
unrelated
here. We do such when we touch lines (or maybe adjacent lines) anyway,
but here you had no need to touch either declaration or the header of
the definition.
Ok. I can make this a separate patch, since I'm already touching the
function
--- a/xen/common/shutdown.c
+++ b/xen/common/shutdown.c
@@ -30,7 +30,7 @@ static void noreturn maybe_reboot(void)
}
}
-void hwdom_shutdown(u8 reason)
+void hwdom_shutdown(uint8_t reason)
{
switch ( reason )
{
@@ -38,39 +38,32 @@ void hwdom_shutdown(u8 reason)
printk("Hardware Dom%u halted: halting machine\n",
hardware_domain->domain_id);
machine_halt();
- break; /* not reached */
case SHUTDOWN_crash:
debugger_trap_immediate();
printk("Hardware Dom%u crashed: ",
hardware_domain->domain_id);
kexec_crash(CRASHREASON_HWDOM);
maybe_reboot();
- break; /* not reached */
Given the function's name, I don't view this removal (and the similar
one
further down) as acceptable. The function first needs renaming so that
from its name one can no longer (wrongly) infer that it may reboot or
else
return.
Jan
Something like "reboot_or_halt" would be okay?
--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)