Re: [Xen-devel] [PATCH] x86/pv: Rename pv/ro-page-fault.c to pv/emul-ro-page-fault.c

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 05/02/18 13:01, Jan Beulich wrote:
>>>> On 05.02.18 at 13:22, <andrew.cooper3@xxxxxxxxxx> wrote:
>> On 05/02/18 08:57, Jan Beulich wrote:
>>>>>> On 02.02.18 at 17:58, <andrew.cooper3@xxxxxxxxxx> wrote:
>>>> To match all our other emulation handling.
>>>>
>>>> No functional change.
>>>>
>>>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>>>> ---
>>>> CC: Jan Beulich <JBeulich@xxxxxxxx>
>>>> ---
>>>>  xen/arch/x86/pv/Makefile                                  | 2 +-
>>>>  xen/arch/x86/pv/{ro-page-fault.c => emul-ro-page-fault.c} | 2 +-
>>>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>>>  rename xen/arch/x86/pv/{ro-page-fault.c => emul-ro-page-fault.c} (99%)
>>> When this file was introduced, iirc I had specifically asked to drop
>>> the pointless emul- prefix. If you want to make things consistent
>>> again, please instead drop the emul- prefixes of the other files.
>> No.
>>
>> First of all, this file is the most recent to come into existence,
>> around 3 months after the others.
> Right - it was too late for me to realize the needlessly long names
> in those earlier code movement patches.

That is a very subjective point of view which I don't agree with.

Naming is all to do with conveying meaning, and shorter isn't
necessarily better.

>
>> The point of naming things in a consistent fashion is for the benefit of
>> humans, and having the emulation related functionality logically grouped
>> is a benefit, not a detriment.
> They're all quite well grouped now already by being in pv/.

That is not the relevant grouping.  Most of our emulation based logic
has an emul- prefix and this file is an odd one out.

Naming the files without their emul- prefix leaves them with no context
as to what they are doing.  "gate-op.c" or "invl-op.c" are far less
obvious to their purpose than "emul-gate-op.c" and "emul-invl-op.c".

> Otherwise do you mean to also change e.g. gpr_switch.S to emul-gpr_switch.S?

This is extra special, and as soon as I can figure out how it actually
works, I plan to replace it with something comprehensive.

~Andrew

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