On 4/16/19 2:51 PM, Andrew Cooper wrote:
> 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.
This is exactly why I did it this way.
The other pattern I think is tolerable is having a #define to use as an
argument; for example:
#define AP2MGET_prepopulate true
#define AP2MGET_peek false
Then you get:
mfn = altp2m_get_entry(..., AP2MGET_prepopulate);
But then you run into namespacing issues with the prefixes.
I wouldn't object to doing it the above way, but I think the wrappers is
probably simpler and clearer for this case. I do object to passing in
naked booleans.
-George
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
