On 16/04/2019 14:44, Tamas K Lengyel wrote:
>
>> diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
>> index 2801a8ccca..8dc4353645 100644
>> --- a/xen/include/asm-x86/p2m.h
>> +++ b/xen/include/asm-x86/p2m.h
>> @@ -514,6 +514,23 @@ static inline unsigned long mfn_to_gfn(struct domain 
>> *d, mfn_t mfn)
>>          return mfn_x(mfn);
>>  }
>>
>> +int altp2m_get_entry(struct p2m_domain *ap2m, gfn_t gfn, mfn_t *mfn,
>> +                     p2m_type_t *t, p2m_access_t *a, bool prepopulate);
>> +
>> +static inline int altp2m_get_entry_direct(struct p2m_domain *ap2m,
>> +                                          gfn_t gfn, mfn_t *mfn,
>> +                                          p2m_type_t *t, p2m_access_t *a)
>> +{
>> +    return altp2m_get_entry(ap2m, gfn, mfn, t, a, false);
>> +}
>> +
>> +static inline int altp2m_get_entry_prepopulate(struct p2m_domain *ap2m,
>> +                                               gfn_t gfn, mfn_t *mfn,
>> +                                               p2m_type_t *t, p2m_access_t 
>> *a)
>> +{
>> +    return altp2m_get_entry(ap2m, gfn, mfn, t, a, true);
>> +}
> Are these wrappers really required? I don't think they add anything to
> readability, it's just yet another layer that does almost nothing.

From a readability point of view, boolean parameters are about as opaque
as they come.  The same goes for all other scalar parameters which don't
have a mnemonic.

For someone who is not an expert of the intricacies of the subsystem,
having the options spelt out in wrappers like this is extremely helpful
to reduce the cognitive load of trying to follow the code.

If it were up to me, all of our code would be wrapped like this, but I
know that others have differing opinions.

~Andrew

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

Reply via email to