Re: [XEN PATCH v6 06/31] x86/mm: avoid building multiple .o from a single .c file

[Jan Beulich <jbeulich@xxxxxxxx>]

On 12.07.2021 14:49, Anthony PERARD wrote:
> On Wed, Jul 07, 2021 at 04:45:11PM +0200, Jan Beulich wrote:
>> On 01.07.2021 16:09, Anthony PERARD wrote:
>>>  xen/Makefile                            | 11 -----------
>>>  xen/Rules.mk                            |  2 +-
>>>  xen/arch/x86/mm/Makefile                |  9 ---------
>>>  xen/arch/x86/mm/guest_walk.c            |  3 ---
>>>  xen/arch/x86/mm/guest_walk_2.c          |  2 ++
>>>  xen/arch/x86/mm/guest_walk_3.c          |  2 ++
>>>  xen/arch/x86/mm/guest_walk_4.c          |  2 ++
>>>  xen/arch/x86/mm/hap/Makefile            |  9 ---------
>>>  xen/arch/x86/mm/hap/guest_walk.c        |  3 ---
>>>  xen/arch/x86/mm/hap/guest_walk_2level.c |  2 ++
>>>  xen/arch/x86/mm/hap/guest_walk_3level.c |  2 ++
>>>  xen/arch/x86/mm/hap/guest_walk_4level.c |  2 ++
>>
>> Is there a particular reason you've kept the "level" in these three
>> file names? Preferably with them shortened (and ideally dashes used
>> everywhere in the new file names instead of underscores)
> 
> I had no reason to rename them. (Also, renaming them makes the patch
> bigger ;-) as we would rename the objects files.)
> 
> But I can rename as part of the patch. Should I rename all of them (mm
> hap and shadow) to "guest-walk-$level.c" ? Or just "guest-$level.c" like
> in shadow/ ? On the other hand, it would probably be helpful to have the same
> basename for both the source and the .c that includes the source (e.g.
> "guest_walk.c" and "guest_walk_2.c" have "guest_walk" in common). So if
> we were to replace underscores by dashes, we should probably rename
> "guest_walk.c" to "guest-walk.c" as well.
> 
> So I'll remove the "level" from the filenames in hap/ to start with in
> an update to this patch, it it worth it to do more that that?

I'd be okay with just the removal of "level"; switching to dashes would
be nice imo, but I can see this being controversial - as you say, names
would go slightly out of sync, and whether renaming the base files is
warranted is hard to say. I'd be personally in favor, but I can see why
people may prefer not to have those extra renames in here.

Jan