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

[Jani Nikula <jani.nikula@linux.intel.com>]

On Wed, 29 May 2024, Michal Wajdeczko <michal.wajdeczko@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.

- 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.

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

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.

> 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...

BR,
Jani.


-- 
Jani Nikula, Intel