Re: [Xen-devel] [PATCH v5 3/5] p2m: change write_p2m_entry to return an error code

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 25.02.19 at 18:42, <george.dunlap@xxxxxxxxxx> wrote:
> On 2/21/19 4:50 PM, Roger Pau Monne wrote:
>> @@ -250,18 +253,37 @@ p2m_next_level(struct p2m_domain *p2m, void **table,
>>          {
>>              new_entry = l1e_from_pfn(pfn | (i << ((level - 1) * 
>> PAGETABLE_ORDER)),
>>                                       flags);
>> -            p2m->write_p2m_entry(p2m, gfn, l1_entry + i, new_entry, level);
>> +            rc = p2m->write_p2m_entry(p2m, gfn, l1_entry + i, new_entry, 
>> level);
>> +            if ( rc )
>> +            {
>> +                ASSERT_UNREACHABLE();
>> +                break;
>> +            }
>>          }
>>  
>>          unmap_domain_page(l1_entry);
>>  
>> -        new_entry = l1e_from_mfn(mfn, P2M_BASE_FLAGS | _PAGE_RW);
>> -        p2m_add_iommu_flags(&new_entry, level, 
>> IOMMUF_readable|IOMMUF_writable);
>> -        p2m->write_p2m_entry(p2m, gfn, p2m_entry, new_entry, level + 1);
>> +        if ( !rc )
>> +        {
>> +            new_entry = l1e_from_mfn(mfn, P2M_BASE_FLAGS | _PAGE_RW);
>> +            p2m_add_iommu_flags(&new_entry, level,
>> +                                IOMMUF_readable|IOMMUF_writable);
>> +            rc = p2m->write_p2m_entry(p2m, gfn, p2m_entry, new_entry,
>> +                                      level + 1);
>> +            if ( rc )
>> +                ASSERT_UNREACHABLE();
>> +        }
>>      }
>>      else
>>          ASSERT(flags & _PAGE_PRESENT);
>>  
>> +    if ( rc )
>> +    {
>> +        ASSERT(mfn_valid(mfn));
>> +        p2m_free_ptp(p2m, mfn_to_page(mfn));
>> +        return rc;
>> +    }
>> +
> 
> I think the idiomatic way to deal with this would be to have a label at
> the end that everything jumps to something like the attached?  That way
> you don't have to spend mental effort making sure that nothing happens
> between the error and the clean-up call.

Well, as Roger has said already, this not being a classical
end-of-the-function error path, I did ask to get away without a
label. But you're the maintainer of the code, so if you want it
that way, then I guess I did misguide Roger.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel