Re: [XEN PATCH 1/2] x86/p2m: preparation work for xenmem_add_to_physmap_one()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 04.12.2023 16:42, Federico Serafini wrote:
> On 04/12/23 15:51, Jan Beulich wrote:
>> On 30.11.2023 16:48, Federico Serafini wrote:
>>> The objective is to use parameter name "gfn" for
>>> xenmem_add_to_physmap_one().
>>> Since the name "gfn" is currently used as identifier for a local
>>> variable, bad things could happen if new uses of such variable are
>>> committed while a renaming patch is waiting for the approval.
>>> To avoid such danger, as first thing rename the local variable from
>>> "gfn" to "gmfn".
>>
>> "..., in line with XENMAPSPACE_gmfn which is the only case it is used
>> with."
>>
>> This is to justify the name not matching our generally aimed at "gfn"
>> and "mfn" scheme.
>>
>>> No functional change.
>>>
>>> Signed-off-by: Federico Serafini <federico.serafini@xxxxxxxxxxx>
>>
>> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> There is an use of "gfn" also few lines outside of the
> switch statement, within an if condition where also XENMAPSPACE_gmfn is
> involved:
> what is true is that "gfn" is used only when space == XENMAPSPACE_gmfn.

Well, sure - me saying "case" wasn't meant to limit things to the switch()
statement.

> What do you think about improve the description by adding:
> "..., in line with XENMAPSPACE_gmfn which is the only *space* it is used
> with."

Fine with me.

> However, the description improvement can be done on commit?

It can. Nevertheless you want to avoid getting into the habit of asking
for things to be done while committing. Strictly speaking on-commit
editing isn't entirely correct, as committing ought to be a purely
mechanical operation. In how far a particular committer is willing to
deviate from that should be left to them.

Jan