Re: [XEN PATCH 11/11] x86/mm: Add assertion to address MISRA C:2012 Rule 2.1

[Nicola Vetrini <nicola.vetrini@bugseng.com>]

On 03/08/2023 11:20, Jan Beulich wrote:
> On 02.08.2023 16:38, Nicola Vetrini wrote:
>> --- a/xen/arch/x86/mm.c
>> +++ b/xen/arch/x86/mm.c
>> @@ -4879,6 +4879,7 @@ long arch_memory_op(unsigned long cmd, 
>> XEN_GUEST_HANDLE_PARAM(void) arg)
>>          return subarch_memory_op(cmd, arg);
>>      }
>> 
>> +    ASSERT_UNREACHABLE();
>>      return 0;
>>  }
> 
> I'd prefer to instead switch earlier "return 0" to "break".

Ok

> 
>> --- a/xen/arch/x86/mm/p2m-pod.c
>> +++ b/xen/arch/x86/mm/p2m-pod.c
>> @@ -1045,6 +1045,7 @@ p2m_pod_zero_check(struct p2m_domain *p2m, const 
>> gfn_t *gfns, unsigned int count
>>      }
>> 
>>      return;
>> +    ASSERT_UNREACHABLE();
>> 
>>  out_unmap:
>>      /*
> 
> In the description you say "before", but here you add something _after_
> "return". What's the deal?
> 
> Jan

In this case the unreachable part is that after the label (looking at it 
now, I should have
put the assert after the label to make it clear), because earlier all 
jumps to
'out_unmap' are like this:

   ASSERT_UNREACHABLE();
   domain_crash(d);
   goto out_unmap;

As I understood it, this is a defensive coding measure, preventing pages 
to remain mapped if,
for some reason the above code actually executes. Am I correct?

Regards,

-- 
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)