On 03.08.2023 11:30, Nicola Vetrini wrote:
> On 03/08/2023 11:20, Jan Beulich wrote:
>> On 02.08.2023 16:38, Nicola Vetrini wrote:
>>> --- 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?
> 
> 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?

The comment there says "probably". So the label and following code might
be used for another error condition as well. Furthermore both paths
presently using "goto out_unmap" already have ASSERT_UNREACHABLE(), so
it's hard to see why we would need yet one more.

Jan

Reply via email to