> On Feb 7, 2022, at 9:38 AM, Jan Beulich <[email protected]> wrote:
> 
> On 04.02.2022 23:07, George Dunlap wrote:
>> On Mon, Jul 5, 2021 at 5:06 PM Jan Beulich <[email protected]> wrote:
>> 
>>> p2m_add_page() is simply a rename from guest_physmap_add_entry().
>>> p2m_remove_page() then is its counterpart, despite rendering
>>> guest_physmap_remove_page().
> 
> First of all: It has been long ago that I noticed that this sentence
> misses words. It now ends "...  a trivial wrapper."
> 
>>> This way callers can use suitable pairs of
>>> functions (previously violated by hvm/grant_table.c).
>>> 
>> 
>> Obviously this needs some clarification.  While we're here, I find this a
>> bit confusing; I tend to use the present tense for the way the code is
>> before the patch, and the imperative for what the patch does; so Id' say:
>> 
>> Rename guest_physmap_add_entry() to p2m_add_page; make
>> guest_physmap_remove_page a wrapper with p2m_remove_page.  That way callers
>> can use suitable pairs...
> 
> Well, yes, I understand you might word it this way. I'm not convinced
> of the fixed scheme you mention for present vs imperative use to be a
> universal fit though, requiring to always be followed. When reading
> the description with the title in mind (and with the previously missing
> words added), I find the use of present tense quite reasonable here.

The way you wrote it is ambiguous grammatically; it could either mean, “Right 
now p2m_add_page() is simply a rename, and so…” or it could mean, “In this 
patch, p2m_add_page() is simply a rename.”  If a reader starts interpreting it 
the first way, then they’ll read along until it doesn’t make sense any more, 
then have to re-evaluate the whole paragraph.

It seems to me that my proposal is unambiguous.

> I'm further slightly puzzled by you keeping the use of present tense in
> "That way callers can use ...".

I wouldn’t call that the present tense; I’m sure a real linguist would have a 
name for it. Consider the sentence, “Put the box near the door; that way we can 
find it easily when we need it.”  The second half of the sentence is set in the 
hypothetical universe in which the imperative has been carried out.

 -George


Reply via email to