Re: [PATCH] docs/devel: Document conventional file prefixes and suffixes

[Stefan Hajnoczi <stefanha@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 4062 bytes --]

On Tue, Dec 26, 2023 at 04:04:41PM +0100, Philippe Mathieu-Daudé wrote:
> Some header and source file names use common prefix / suffix
> but we never really ruled a convention. Start doing so with
> the current patterns from the tree.
> 
> Suggested-by: Alex Bennée <alex.bennee@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  docs/devel/style.rst | 49 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 49 insertions(+)
> 
> diff --git a/docs/devel/style.rst b/docs/devel/style.rst
> index 2f68b50079..4da50eb2ea 100644
> --- a/docs/devel/style.rst
> +++ b/docs/devel/style.rst
> @@ -162,6 +162,55 @@ pre-processor. Another common suffix is ``_impl``; it is used for the
>  concrete implementation of a function that will not be called
>  directly, but rather through a macro or an inline function.
>  
> +File Naming Conventions
> +-----------------------
> +
> +Public headers
> +~~~~~~~~~~~~~~
> +
> +Headers expected to be access by multiple subsystems must reside in

s/access/accessed/

> +the ``include/`` folder. Headers local to a subsystem should reside in
> +the sysbsystem folder, if any (for example ``qobject/qobject-internal.h``

s/sysbsystem/subsystem/

> +can only be included by files within the ``qobject/`` folder).
> +
> +Header file prefix and suffix hints
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +When headers relate to common concept, it is useful to use a common

Either "common concepts" (plural) or "a common concept" (singular with
an indefinite article).

> +prefix or suffix.
> +
> +When headers relate to the same (guest) subsystem, the subsystem name is
> +often used as prefix. If headers are already in a folder named as the
> +subsystem, prefixing them is optional.

"named as the subsystem" sounds strange. I suggest something like:

"If headers are already in a folder with the subsystem in its name,
prefixing them is optional."

or

"Prefixing header files is optional if the folder name already contains
the subsystem name."

> +
> +For example, hardware models related to the Aspeed systems are named
> +using the ``aspeed_`` prefix.
> +
> +Headers related to the same (host) concept can also use a common prefix.

Is there a need to distinguish between "(guest)" above and "(host)" here
since we end up recommending the same thing for both?

> +For example OS specific headers use the ``-posix`` and ``-win32`` suffixes.

The previous sentence is about prefixes but this sentence focusses on
suffixes. That's a little confusing. I guess you mean "foo-posix" and
"foo-win32" have a common prefix. It may help to express it in terms of
the prefix instead of mentioning the suffix.

> +
> +Registered file suffixes
> +~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +* ``.inc``
> +
> +  Source files meant to be included by other source files as templates
> +  must use the ``.c.inc`` suffix. Similarly, headers meant to be included
> +  multiple times as template must use the ``.h.inc`` suffix.
> +
> +Recommended file prefixes / suffixes
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +* ``target`` and ``common`` suffixes
> +
> +  Files which are specific to a target should use the ``target`` suffix.
> +  Such ``target`` suffixed headers usually *taint* the files including them
> +  by making them target specific.

Is there any particular macro or pattern for enforcing this? I remember
there are #error preprocessor directives in some header files to prevent
including them from the wrong source file, but I'm not sure if you're
referring to anything specific here.

> +
> +  Files common to all targets should use the ``common`` suffix, to provide
> +  a hint that these files can be safely included from common code.

This statement is too general. For example, files in util/ can be used
from common code but don't have a suffix. I think target and common
suffixes are useful when something is split into target-specific and
common parts. Otherwise it's not necessary.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]