[PATCH 4/5] drm/xe: Rename internal vram helper function

[Michal Wajdeczko]

On 29.05.2024 13:50, Jani Nikula wrote:
> On Wed, 29 May 2024, Michal Wajdeczko <michal.wajdeczko at intel.com> wrote:
>> ++ maintainers
>>
>> On 29.05.2024 00:15, Matthew Brost wrote:
>>> We have talked about this before and I think the consensous was "xe_"
>>> prefixes for static functions are fine, perhaps even desired. I can't
>>
>> IMO use of "xe_foo" prefix for static functions is inconsistent (as its
>> name suggests that it is public function while OTOH it is declared as
>> static) and may mislead that it could be used in other compilation unit.
>>
>> thus use of "xe_" (for static) should be rather discouraged but also at
>> the same time it shouldn't be treated as showstopper (with the hope
>> better name will be provided later)
>>
>>> find a reference but I do recall a discussion on the list about this.
>>>
>>> I think the maintainers should make / document a rule wrt to "xe_"
>>> prefixes before we start making changes in this area as it is not clear.
> 
> IMO the prefix is useful even for static functions for a few reasons:
> 
> - C has no namespaces. The use of prefixes in the kernel global
>   functions and macros is inconsistent at best. Prefixes avoid clashes.

good point but I guess any new global function will be named in a way
that will minimize risk of clash

> 
> - It's quite handy to be able to just glance at a backtrace to see where
>   it's originating from. With a bunch of generic non-prefixed functions
>   in there, you kind of lose track, and have to look at the source.

if you are unlucky and not all static functions were inlined then still
the backtrace shall include top level public xe_foo() function

> 
> - During refactoring, it's not uncommon to make functions
>   static/non-static, and having to rename at that point is a bit
>   tedious.

another good reason to avoid refactoring and come with good design in
the first place ;)

> 
> That said, we haven't required this in i915. Also platform acronym
> prefixes are common especially in display code. Some people have started
> adding single underscore prefixes for static functions in a few places,
> but I pretty much frown on that.

and sometimes static functions names are defined/added to clarify the
flow or function logic, and strict adhere to rules to have a prefix
might make it less clear, so some room for common sense is still needed

> 
>> before we start writing rules for static functions, better to focus only
>> on rules for public naming convention for the components, like:
>>
>> files:
>> 	GOOD
>> 		xe_foo.ch
>> 		xe_foo_types.h
>> 		xe_foo_helpers.h
>> 	FINE
>> 		xe_gt_foo.ch
>> 	BAD
>> 		foo.ch
>>
>> types:
>> 	GOOD
>> 		struct xe_foo_xxx
>> 		enum xe_foo_yyy
>> 		typedef xe_foo_zzz
>> 	BAD
>> 		struct foo
>> 		struct xe_bar
>>
>> functions:
>> 	GOOD
>> 		xe_foo_bar(struct xe_foo *foo, ...)
>> 	FINE
>> 		xe_foo_bar(struct xe_device *xe, ...)
>> 		xe_gt_foo_bar(struct xe_gt *gt, ...)
>> 	BAD
>> 		xe_foo_bar(..., struct xe_foo *foo)
>> 		xe_bar_foo(struct xe_foo *foo, ...)
> 
> And sometimes you want to prefer names that are easy to pronounce and
> make sense over names that strictly adhere to rules...

but then it should be treated as an exception, to be approved by the
maintainer, from general guidelines that maintainers can still document

> 
> BR,
> Jani.
> 
>