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.
> --- 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
