[PATCH 1/3] xen/keyhandler: add missing noreturn attribute

[PATCH 1/3] xen/keyhandler: add missing noreturn attribute

[victorm.lira]

From: Nicola Vetrini <nicola.vetrini@bugseng.com>

Function `reboot_machine' does not return, but lacks the `noreturn' attribute,
therefore causing a violation of MISRA C Rule 2.1: "A project shall not contain
unreachable code".

No functional change.

Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Signed-off-by: Victor Lira <victorm.lira@amd.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Anthony PERARD <anthony.perard@vates.tech>
Cc: Michal Orzel <michal.orzel@amd.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Julien Grall <julien@xen.org>
Cc: Roger Pau Monné <roger.pau@citrix.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Nicola Vetrini <nicola.vetrini@bugseng.com>
Cc: Federico Serafini <federico.serafini@bugseng.com>
Cc: Bertrand Marquis <bertrand.marquis@arm.com>
---

 xen/common/keyhandler.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/common/keyhandler.c b/xen/common/keyhandler.c
index 0bb842ec00..b0a2051408 100644
--- a/xen/common/keyhandler.c
+++ b/xen/common/keyhandler.c
@@ -251,7 +251,7 @@ static void cf_check dump_hwdom_registers(unsigned char key)
     }
 }

-static void cf_check reboot_machine(unsigned char key, bool unused)
+static void noreturn cf_check reboot_machine(unsigned char key, bool unused)
 {
     printk("'%c' pressed -> rebooting machine\n", key);
     machine_restart(0);
--
2.25.1

[PATCH 2/3] xen/arm: add missing noreturn attributes

[victorm.lira]

From: Nicola Vetrini <nicola.vetrini@bugseng.com>

The marked functions never return to their caller, but
lack the `noreturn' attribute, therefore causing a violation
of MISRA C Rule 2.1: "A project shall not contain unreachable code".

No functional change.

Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Signed-off-by: Victor Lira <victorm.lira@amd.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Anthony PERARD <anthony.perard@vates.tech>
Cc: Michal Orzel <michal.orzel@amd.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Julien Grall <julien@xen.org>
Cc: Roger Pau Monné <roger.pau@citrix.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Nicola Vetrini <nicola.vetrini@bugseng.com>
Cc: Federico Serafini <federico.serafini@bugseng.com>
Cc: Bertrand Marquis <bertrand.marquis@arm.com>
---

 xen/arch/arm/efi/efi-boot.h            | 2 +-
 xen/arch/arm/include/asm/arm64/traps.h | 2 +-
 xen/arch/arm/include/asm/processor.h   | 2 +-
 xen/arch/arm/setup.c                   | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/xen/arch/arm/efi/efi-boot.h b/xen/arch/arm/efi/efi-boot.h
index d2a09ad3a1..ee80560e13 100644
--- a/xen/arch/arm/efi/efi-boot.h
+++ b/xen/arch/arm/efi/efi-boot.h
@@ -934,7 +934,7 @@ static void __init efi_arch_blexit(void)
         efi_bs->FreePool(memmap);
 }

-static void __init efi_arch_halt(void)
+static void noreturn __init efi_arch_halt(void)
 {
     stop_cpu();
 }
diff --git a/xen/arch/arm/include/asm/arm64/traps.h b/xen/arch/arm/include/asm/arm64/traps.h
index 3be2fa69ee..b7435c6e73 100644
--- a/xen/arch/arm/include/asm/arm64/traps.h
+++ b/xen/arch/arm/include/asm/arm64/traps.h
@@ -6,7 +6,7 @@ void inject_undef64_exception(struct cpu_user_regs *regs);
 void do_sysreg(struct cpu_user_regs *regs,
                const union hsr hsr);

-void do_bad_mode(struct cpu_user_regs *regs, int reason);
+void noreturn do_bad_mode(struct cpu_user_regs *regs, int reason);

 #endif /* __ASM_ARM64_TRAPS__ */
 /*
diff --git a/xen/arch/arm/include/asm/processor.h b/xen/arch/arm/include/asm/processor.h
index 9cbc4f9110..92c8bc1a31 100644
--- a/xen/arch/arm/include/asm/processor.h
+++ b/xen/arch/arm/include/asm/processor.h
@@ -571,7 +571,7 @@ extern register_t __cpu_logical_map[];
 #endif

 #ifndef __ASSEMBLY__
-void panic_PAR(uint64_t par);
+void noreturn panic_PAR(uint64_t par);

 /* Debugging functions are declared with external linkage to aid development. */
 void show_registers(const struct cpu_user_regs *regs);
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 734e23da44..ed72317af3 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -63,7 +63,7 @@ bool __read_mostly acpi_disabled;

 domid_t __read_mostly max_init_domid;

-static __used void init_done(void)
+static __used void noreturn init_done(void)
 {
     int rc;

--
2.25.1

[PATCH 3/3] xen/x86: add missing noreturn attributes

[victorm.lira]

From: Nicola Vetrini <nicola.vetrini@bugseng.com>

The marked functions never return to their caller, but
lack the `noreturn' attribute, therefore causing a violation
of MISRA C Rule 2.1: "A project shall not contain unreachable code".

No functional change.

Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Signed-off-by: Victor Lira <victorm.lira@amd.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Anthony PERARD <anthony.perard@vates.tech>
Cc: Michal Orzel <michal.orzel@amd.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Julien Grall <julien@xen.org>
Cc: Roger Pau Monné <roger.pau@citrix.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Nicola Vetrini <nicola.vetrini@bugseng.com>
Cc: Federico Serafini <federico.serafini@bugseng.com>
Cc: Bertrand Marquis <bertrand.marquis@arm.com>
---

 xen/arch/x86/cpu/mcheck/mce.c | 3 ++-
 xen/arch/x86/efi/efi-boot.h   | 2 +-
 xen/arch/x86/smp.c            | 2 +-
 xen/arch/x86/traps.c          | 2 +-
 xen/arch/x86/x86_64/traps.c   | 2 +-
 5 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/xen/arch/x86/cpu/mcheck/mce.c b/xen/arch/x86/cpu/mcheck/mce.c
index 1c348e557d..79214ce56b 100644
--- a/xen/arch/x86/cpu/mcheck/mce.c
+++ b/xen/arch/x86/cpu/mcheck/mce.c
@@ -79,7 +79,8 @@ static int __init cf_check mce_set_verbosity(const char *str)
 custom_param("mce_verbosity", mce_set_verbosity);

 /* Handle unconfigured int18 (should never happen) */
-static void cf_check unexpected_machine_check(const struct cpu_user_regs *regs)
+static void noreturn cf_check
+unexpected_machine_check(const struct cpu_user_regs *regs)
 {
     console_force_unlock();
     printk("Unexpected Machine Check Exception\n");
diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
index 0ecf4ca53f..0194720003 100644
--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -769,7 +769,7 @@ static void __init efi_arch_blexit(void)
         efi_bs->FreePages(ucode.addr, PFN_UP(ucode.size));
 }

-static void __init efi_arch_halt(void)
+static void noreturn __init efi_arch_halt(void)
 {
     local_irq_disable();
     for ( ; ; )
diff --git a/xen/arch/x86/smp.c b/xen/arch/x86/smp.c
index 516dab5528..7936294f5f 100644
--- a/xen/arch/x86/smp.c
+++ b/xen/arch/x86/smp.c
@@ -343,7 +343,7 @@ void __stop_this_cpu(void)
     cpumask_clear_cpu(smp_processor_id(), &cpu_online_map);
 }

-static void cf_check stop_this_cpu(void *dummy)
+static void noreturn cf_check stop_this_cpu(void *dummy)
 {
     const bool *stop_aps = dummy;

diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 092c7e4197..34dc077cad 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -805,7 +805,7 @@ void fatal_trap(const struct cpu_user_regs *regs, bool show_remote)
           (regs->eflags & X86_EFLAGS_IF) ? "" : " IN INTERRUPT CONTEXT");
 }

-void asmlinkage do_unhandled_trap(struct cpu_user_regs *regs)
+void asmlinkage noreturn do_unhandled_trap(struct cpu_user_regs *regs)
 {
     fatal_trap(regs, false);
 }
diff --git a/xen/arch/x86/x86_64/traps.c b/xen/arch/x86/x86_64/traps.c
index c77f304bb0..8460a4a1ae 100644
--- a/xen/arch/x86/x86_64/traps.c
+++ b/xen/arch/x86/x86_64/traps.c
@@ -293,7 +293,7 @@ void show_page_walk(unsigned long addr)
            l1_table_offset(addr), l1e_get_intpte(l1e), pfn);
 }

-void asmlinkage do_double_fault(struct cpu_user_regs *regs)
+void asmlinkage noreturn do_double_fault(struct cpu_user_regs *regs)
 {
     unsigned int cpu;
     struct extra_state state;
--
2.25.1

Re: [PATCH 1/3] xen/keyhandler: add missing noreturn attribute

[Jan Beulich]

On 05.06.2025 01:49, victorm.lira@amd.com wrote:
> From: Nicola Vetrini <nicola.vetrini@bugseng.com>
> 
> Function `reboot_machine' does not return, but lacks the `noreturn' attribute,
> therefore causing a violation of MISRA C Rule 2.1: "A project shall not contain
> unreachable code".

Is this (uniformly) true? Looking at ...

> --- a/xen/common/keyhandler.c
> +++ b/xen/common/keyhandler.c
> @@ -251,7 +251,7 @@ static void cf_check dump_hwdom_registers(unsigned char key)
>      }
>  }
> 
> -static void cf_check reboot_machine(unsigned char key, bool unused)
> +static void noreturn cf_check reboot_machine(unsigned char key, bool unused)
>  {
>      printk("'%c' pressed -> rebooting machine\n", key);
>      machine_restart(0);

... generated code here, I can see that the compiler is perfectly able to
leverage the noreturn that machine_restart() has, resulting in no
unreachable code to be generated. That is - neither in source nor in
binary there is any unreachable code. Therefore I'm having a hard time
seeing what the violation is here.

That said, I certainly don't mind the addition of the (seemingly) missing
attribute. Otoh I wonder whether an attribute the removal of which has no
effect wouldn't count as "dead code" or alike, violating some other rule.

Jan

Re: [PATCH 3/3] xen/x86: add missing noreturn attributes

[Jan Beulich]

On 05.06.2025 01:49, victorm.lira@amd.com wrote:
> From: Nicola Vetrini <nicola.vetrini@bugseng.com>
> 
> The marked functions never return to their caller, but
> lack the `noreturn' attribute, therefore causing a violation
> of MISRA C Rule 2.1: "A project shall not contain unreachable code".
> 
> No functional change.
> 
> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
> Signed-off-by: Victor Lira <victorm.lira@amd.com>

I expect the comment on patch 1 applies to some or all of the cases here,
too. For example, I expect that even if efi_arch_halt() wasn't inlined into
its sole caller, the compiler would also be able to infer noreturn from the
infinite loop there.

Jan

Re: [PATCH 1/3] xen/keyhandler: add missing noreturn attribute

[Nicola Vetrini]

On 2025-06-05 09:17, Jan Beulich wrote:
> On 05.06.2025 01:49, victorm.lira@amd.com wrote:
>> From: Nicola Vetrini <nicola.vetrini@bugseng.com>
>> 
>> Function `reboot_machine' does not return, but lacks the `noreturn' 
>> attribute,
>> therefore causing a violation of MISRA C Rule 2.1: "A project shall 
>> not contain
>> unreachable code".
> 
> Is this (uniformly) true? Looking at ...
> 
>> --- a/xen/common/keyhandler.c
>> +++ b/xen/common/keyhandler.c
>> @@ -251,7 +251,7 @@ static void cf_check dump_hwdom_registers(unsigned 
>> char key)
>>      }
>>  }
>> 
>> -static void cf_check reboot_machine(unsigned char key, bool unused)
>> +static void noreturn cf_check reboot_machine(unsigned char key, bool 
>> unused)
>>  {
>>      printk("'%c' pressed -> rebooting machine\n", key);
>>      machine_restart(0);
> 
> ... generated code here, I can see that the compiler is perfectly able 
> to
> leverage the noreturn that machine_restart() has, resulting in no
> unreachable code to be generated. That is - neither in source nor in
> binary there is any unreachable code. Therefore I'm having a hard time
> seeing what the violation is here.
> 
> That said, I certainly don't mind the addition of the (seemingly) 
> missing
> attribute. Otoh I wonder whether an attribute the removal of which has 
> no
> effect wouldn't count as "dead code" or alike, violating some other 
> rule.
> 

Inlining does not play a role in this case. Here reboot_machine() is 
marked as a violation because machine_restart() is noreturn and there is 
no other path upon which reboot_machine() may return, hence any function 
calling reboot_machine() could have portions that are inadvertently 
unreachable (as in never executed due to divergence) by not having the 
annotation. That said, in such trivial cases compilers are typically 
able to derive the property automatically, but they are not obliged to 
and, more importantly, the behavior may even differ with the same 
compiler using different optimization levels.

-- 
Nicola Vetrini, B.Sc.
Software Engineer
BUGSENG (https://bugseng.com)
LinkedIn: https://www.linkedin.com/in/nicola-vetrini-a42471253

Re: [PATCH 1/3] xen/keyhandler: add missing noreturn attribute

[Nicola Vetrini]

On 2025-06-05 14:22, Nicola Vetrini wrote:
> On 2025-06-05 09:17, Jan Beulich wrote:
>> On 05.06.2025 01:49, victorm.lira@amd.com wrote:
>>> From: Nicola Vetrini <nicola.vetrini@bugseng.com>
>>> 
>>> Function `reboot_machine' does not return, but lacks the `noreturn' 
>>> attribute,
>>> therefore causing a violation of MISRA C Rule 2.1: "A project shall 
>>> not contain
>>> unreachable code".
>> 
>> Is this (uniformly) true? Looking at ...
>> 
>>> --- a/xen/common/keyhandler.c
>>> +++ b/xen/common/keyhandler.c
>>> @@ -251,7 +251,7 @@ static void cf_check 
>>> dump_hwdom_registers(unsigned char key)
>>>      }
>>>  }
>>> 
>>> -static void cf_check reboot_machine(unsigned char key, bool unused)
>>> +static void noreturn cf_check reboot_machine(unsigned char key, bool 
>>> unused)
>>>  {
>>>      printk("'%c' pressed -> rebooting machine\n", key);
>>>      machine_restart(0);
>> 
>> ... generated code here, I can see that the compiler is perfectly able 
>> to
>> leverage the noreturn that machine_restart() has, resulting in no
>> unreachable code to be generated. That is - neither in source nor in
>> binary there is any unreachable code. Therefore I'm having a hard time
>> seeing what the violation is here.
>> 
>> That said, I certainly don't mind the addition of the (seemingly) 
>> missing
>> attribute. Otoh I wonder whether an attribute the removal of which has 
>> no
>> effect wouldn't count as "dead code" or alike, violating some other 
>> rule.
>> 
> 
> Inlining does not play a role in this case. Here reboot_machine() is 
> marked as a violation because machine_restart() is noreturn and there 
> is no other path upon which reboot_machine() may return, hence any 
> function calling reboot_machine() could have portions that are 
> inadvertently unreachable (as in never executed due to divergence) by 
> not having the annotation. That said, in such trivial cases compilers 
> are typically able to derive the property automatically, but they are 
> not obliged to and, more importantly, the behavior may even differ with 
> the same compiler using different optimization levels.

Just a note: in later revisions of MISRA C this has become a rule of its 
own [1], which helps reduce confusion, but up to MISRA C:2012 Amendment 
2 (currently used by Xen), this is part of Rule 2.1.

[1] Rule 17.11: "A function that never returns should be declared with a 
_Noreturn function specifier"

-- 
Nicola Vetrini, B.Sc.
Software Engineer
BUGSENG (https://bugseng.com)
LinkedIn: https://www.linkedin.com/in/nicola-vetrini-a42471253

Re: [PATCH 1/3] xen/keyhandler: add missing noreturn attribute

[Jan Beulich]

On 05.06.2025 14:26, Nicola Vetrini wrote:
> On 2025-06-05 14:22, Nicola Vetrini wrote:
>> On 2025-06-05 09:17, Jan Beulich wrote:
>>> On 05.06.2025 01:49, victorm.lira@amd.com wrote:
>>>> From: Nicola Vetrini <nicola.vetrini@bugseng.com>
>>>>
>>>> Function `reboot_machine' does not return, but lacks the `noreturn' 
>>>> attribute,
>>>> therefore causing a violation of MISRA C Rule 2.1: "A project shall 
>>>> not contain
>>>> unreachable code".
>>>
>>> Is this (uniformly) true? Looking at ...
>>>
>>>> --- a/xen/common/keyhandler.c
>>>> +++ b/xen/common/keyhandler.c
>>>> @@ -251,7 +251,7 @@ static void cf_check 
>>>> dump_hwdom_registers(unsigned char key)
>>>>      }
>>>>  }
>>>>
>>>> -static void cf_check reboot_machine(unsigned char key, bool unused)
>>>> +static void noreturn cf_check reboot_machine(unsigned char key, bool 
>>>> unused)
>>>>  {
>>>>      printk("'%c' pressed -> rebooting machine\n", key);
>>>>      machine_restart(0);
>>>
>>> ... generated code here, I can see that the compiler is perfectly able 
>>> to
>>> leverage the noreturn that machine_restart() has, resulting in no
>>> unreachable code to be generated. That is - neither in source nor in
>>> binary there is any unreachable code. Therefore I'm having a hard time
>>> seeing what the violation is here.
>>>
>>> That said, I certainly don't mind the addition of the (seemingly) 
>>> missing
>>> attribute. Otoh I wonder whether an attribute the removal of which has 
>>> no
>>> effect wouldn't count as "dead code" or alike, violating some other 
>>> rule.
>>>
>>
>> Inlining does not play a role in this case. Here reboot_machine() is 
>> marked as a violation because machine_restart() is noreturn and there 
>> is no other path upon which reboot_machine() may return, hence any 
>> function calling reboot_machine() could have portions that are 
>> inadvertently unreachable (as in never executed due to divergence) by 
>> not having the annotation.

Just that there's not going to be a 2nd caller, considering the purpose
of the function.

>> That said, in such trivial cases compilers 
>> are typically able to derive the property automatically, but they are 
>> not obliged to and, more importantly, the behavior may even differ with 
>> the same compiler using different optimization levels.
> 
> Just a note: in later revisions of MISRA C this has become a rule of its 
> own [1], which helps reduce confusion, but up to MISRA C:2012 Amendment 
> 2 (currently used by Xen), this is part of Rule 2.1.
> 
> [1] Rule 17.11: "A function that never returns should be declared with a 
> _Noreturn function specifier"

Oh, that's indeed quite a bit more explicit.

Jan

Re: [PATCH 1/3] xen/keyhandler: add missing noreturn attribute

[Stefano Stabellini]

On Thu, 5 Jun 2025, Jan Beulich wrote:
> On 05.06.2025 14:26, Nicola Vetrini wrote:
> > On 2025-06-05 14:22, Nicola Vetrini wrote:
> >> On 2025-06-05 09:17, Jan Beulich wrote:
> >>> On 05.06.2025 01:49, victorm.lira@amd.com wrote:
> >>>> From: Nicola Vetrini <nicola.vetrini@bugseng.com>
> >>>>
> >>>> Function `reboot_machine' does not return, but lacks the `noreturn' 
> >>>> attribute,
> >>>> therefore causing a violation of MISRA C Rule 2.1: "A project shall 
> >>>> not contain
> >>>> unreachable code".
> >>>
> >>> Is this (uniformly) true? Looking at ...
> >>>
> >>>> --- a/xen/common/keyhandler.c
> >>>> +++ b/xen/common/keyhandler.c
> >>>> @@ -251,7 +251,7 @@ static void cf_check 
> >>>> dump_hwdom_registers(unsigned char key)
> >>>>      }
> >>>>  }
> >>>>
> >>>> -static void cf_check reboot_machine(unsigned char key, bool unused)
> >>>> +static void noreturn cf_check reboot_machine(unsigned char key, bool 
> >>>> unused)
> >>>>  {
> >>>>      printk("'%c' pressed -> rebooting machine\n", key);
> >>>>      machine_restart(0);
> >>>
> >>> ... generated code here, I can see that the compiler is perfectly able 
> >>> to
> >>> leverage the noreturn that machine_restart() has, resulting in no
> >>> unreachable code to be generated. That is - neither in source nor in
> >>> binary there is any unreachable code. Therefore I'm having a hard time
> >>> seeing what the violation is here.
> >>>
> >>> That said, I certainly don't mind the addition of the (seemingly) 
> >>> missing
> >>> attribute. Otoh I wonder whether an attribute the removal of which has 
> >>> no
> >>> effect wouldn't count as "dead code" or alike, violating some other 
> >>> rule.
> >>>
> >>
> >> Inlining does not play a role in this case. Here reboot_machine() is 
> >> marked as a violation because machine_restart() is noreturn and there 
> >> is no other path upon which reboot_machine() may return, hence any 
> >> function calling reboot_machine() could have portions that are 
> >> inadvertently unreachable (as in never executed due to divergence) by 
> >> not having the annotation.
> 
> Just that there's not going to be a 2nd caller, considering the purpose
> of the function.
> 
> >> That said, in such trivial cases compilers 
> >> are typically able to derive the property automatically, but they are 
> >> not obliged to and, more importantly, the behavior may even differ with 
> >> the same compiler using different optimization levels.
> > 
> > Just a note: in later revisions of MISRA C this has become a rule of its 
> > own [1], which helps reduce confusion, but up to MISRA C:2012 Amendment 
> > 2 (currently used by Xen), this is part of Rule 2.1.
> > 
> > [1] Rule 17.11: "A function that never returns should be declared with a 
> > _Noreturn function specifier"
> 
> Oh, that's indeed quite a bit more explicit.

Does it mean you would ack the patch? :-)

Re: [PATCH 1/3] xen/keyhandler: add missing noreturn attribute

[Jan Beulich]

On 06.06.2025 02:09, Stefano Stabellini wrote:
> On Thu, 5 Jun 2025, Jan Beulich wrote:
>> On 05.06.2025 14:26, Nicola Vetrini wrote:
>>> On 2025-06-05 14:22, Nicola Vetrini wrote:
>>>> On 2025-06-05 09:17, Jan Beulich wrote:
>>>>> On 05.06.2025 01:49, victorm.lira@amd.com wrote:
>>>>>> From: Nicola Vetrini <nicola.vetrini@bugseng.com>
>>>>>>
>>>>>> Function `reboot_machine' does not return, but lacks the `noreturn' 
>>>>>> attribute,
>>>>>> therefore causing a violation of MISRA C Rule 2.1: "A project shall 
>>>>>> not contain
>>>>>> unreachable code".
>>>>>
>>>>> Is this (uniformly) true? Looking at ...
>>>>>
>>>>>> --- a/xen/common/keyhandler.c
>>>>>> +++ b/xen/common/keyhandler.c
>>>>>> @@ -251,7 +251,7 @@ static void cf_check 
>>>>>> dump_hwdom_registers(unsigned char key)
>>>>>>      }
>>>>>>  }
>>>>>>
>>>>>> -static void cf_check reboot_machine(unsigned char key, bool unused)
>>>>>> +static void noreturn cf_check reboot_machine(unsigned char key, bool 
>>>>>> unused)
>>>>>>  {
>>>>>>      printk("'%c' pressed -> rebooting machine\n", key);
>>>>>>      machine_restart(0);
>>>>>
>>>>> ... generated code here, I can see that the compiler is perfectly able 
>>>>> to
>>>>> leverage the noreturn that machine_restart() has, resulting in no
>>>>> unreachable code to be generated. That is - neither in source nor in
>>>>> binary there is any unreachable code. Therefore I'm having a hard time
>>>>> seeing what the violation is here.
>>>>>
>>>>> That said, I certainly don't mind the addition of the (seemingly) 
>>>>> missing
>>>>> attribute. Otoh I wonder whether an attribute the removal of which has 
>>>>> no
>>>>> effect wouldn't count as "dead code" or alike, violating some other 
>>>>> rule.
>>>>>
>>>>
>>>> Inlining does not play a role in this case. Here reboot_machine() is 
>>>> marked as a violation because machine_restart() is noreturn and there 
>>>> is no other path upon which reboot_machine() may return, hence any 
>>>> function calling reboot_machine() could have portions that are 
>>>> inadvertently unreachable (as in never executed due to divergence) by 
>>>> not having the annotation.
>>
>> Just that there's not going to be a 2nd caller, considering the purpose
>> of the function.
>>
>>>> That said, in such trivial cases compilers 
>>>> are typically able to derive the property automatically, but they are 
>>>> not obliged to and, more importantly, the behavior may even differ with 
>>>> the same compiler using different optimization levels.
>>>
>>> Just a note: in later revisions of MISRA C this has become a rule of its 
>>> own [1], which helps reduce confusion, but up to MISRA C:2012 Amendment 
>>> 2 (currently used by Xen), this is part of Rule 2.1.
>>>
>>> [1] Rule 17.11: "A function that never returns should be declared with a 
>>> _Noreturn function specifier"
>>
>> Oh, that's indeed quite a bit more explicit.
> 
> Does it mean you would ack the patch? :-)

With an improved description I may at least no longer object to it.

Jan