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

Philippe Mathieu-Daudé posted 1 patch 11 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20231226150441.97501-1-philmd@linaro.org
Maintainers: "Alex Bennée" <alex.bennee@linaro.org>, "Daniel P. Berrangé" <berrange@redhat.com>, Thomas Huth <thuth@redhat.com>, Markus Armbruster <armbru@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Juan Quintela <quintela@redhat.com>
docs/devel/style.rst | 49 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 49 insertions(+)
[PATCH] docs/devel: Document conventional file prefixes and suffixes
Posted by Philippe Mathieu-Daudé 11 months ago
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
+the ``include/`` folder. Headers local to a subsystem should reside in
+the sysbsystem folder, if any (for example ``qobject/qobject-internal.h``
+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
+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.
+
+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.
+For example OS specific headers use the ``-posix`` and ``-win32`` suffixes.
+
+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.
+
+  Files common to all targets should use the ``common`` suffix, to provide
+  a hint that these files can be safely included from common code.
+
+
 Block structure
 ===============
 
-- 
2.41.0


Re: [PATCH] docs/devel: Document conventional file prefixes and suffixes
Posted by Stefan Hajnoczi 10 months, 4 weeks ago
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.
Re: [PATCH] docs/devel: Document conventional file prefixes and suffixes
Posted by Richard Henderson 11 months ago
On 12/27/23 02:04, 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
> +the ``include/`` folder. Headers local to a subsystem should reside in
> +the sysbsystem folder, if any (for example ``qobject/qobject-internal.h``

subsystem.

r~

Re: [PATCH] docs/devel: Document conventional file prefixes and suffixes
Posted by Zhao Liu 11 months ago
Hi Philippe,

On Tue, Dec 26, 2023 at 04:04:41PM +0100, Philippe Mathieu-Daudé wrote:
> Date: Tue, 26 Dec 2023 16:04:41 +0100
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
> Subject: [PATCH] docs/devel: Document conventional file prefixes and
>  suffixes
> X-Mailer: git-send-email 2.41.0
> 
> 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
> +the ``include/`` folder. Headers local to a subsystem should reside in
> +the sysbsystem folder, if any (for example ``qobject/qobject-internal.h``
> +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
> +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.
> +
> +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.
                                                                    ^^^^^^
                                                             Maybe "suffix"?

since below you provide examples of "suffix".

> +For example OS specific headers use the ``-posix`` and ``-win32`` suffixes.
> +
> +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.

emm, it seems linux-use/* and bsd-user/* have many ``target`` prefix
headers. Should they get cleaned up?


> +  Such ``target`` suffixed headers usually *taint* the files including them
> +  by making them target specific.
> +
> +  Files common to all targets should use the ``common`` suffix, to provide
> +  a hint that these files can be safely included from common code.
> +
> +

An additional question that kind of confuses me is whether header file
naming should use "-" or "_" to connect prefixes/suffixes?

>  Block structure
>  ===============
>  
> -- 
> 2.41.0
> 

Thanks,
Zhao
Re: [PATCH] docs/devel: Document conventional file prefixes and suffixes
Posted by Philippe Mathieu-Daudé 11 months ago
Hi,

On 27/12/23 08:12, Zhao Liu wrote:
> Hi Philippe,
> 
> On Tue, Dec 26, 2023 at 04:04:41PM +0100, Philippe Mathieu-Daudé wrote:
>> Date: Tue, 26 Dec 2023 16:04:41 +0100
>> From: Philippe Mathieu-Daudé <philmd@linaro.org>
>> Subject: [PATCH] docs/devel: Document conventional file prefixes and
>>   suffixes
>> X-Mailer: git-send-email 2.41.0
>>
>> 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
>> +the ``include/`` folder. Headers local to a subsystem should reside in
>> +the sysbsystem folder, if any (for example ``qobject/qobject-internal.h``
>> +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
>> +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.
>> +
>> +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.
>                                                                      ^^^^^^
>                                                               Maybe "suffix"?
> 
> since below you provide examples of "suffix".

Oops, indeed :)

>> +For example OS specific headers use the ``-posix`` and ``-win32`` suffixes.
>> +
>> +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.
> 
> emm, it seems linux-use/* and bsd-user/* have many ``target`` prefix
> headers. Should they get cleaned up?

No, these are special to user emulation, and are defined for each
target, to be included once per target build, i.e. for Linux:

$ git grep '#include "target_' linux-user
linux-user/elfload.c:23:#include "target_signal.h"
linux-user/flatload.c:43:#include "target_flat.h"
linux-user/main.c:50:#include "target_elf.h"
linux-user/mmap.c:26:#include "target_mman.h"
linux-user/qemu.h:12:#include "target_syscall.h"
linux-user/strace.c:21:#include "target_mman.h"
linux-user/syscall.c:27:#include "target_mman.h"
linux-user/syscall.c:6313:#include "target_prctl.h"
linux-user/syscall.c:8252:#include "target_proc.h"
linux-user/syscall_defs.h:497:#include "target_signal.h"
linux-user/syscall_defs.h:701:#include "target_resource.h"
linux-user/syscall_defs.h:1230:#include "target_mman.h"
linux-user/syscall_defs.h:2256:#include "target_fcntl.h"
linux-user/syscall_defs.h:2577:#include "target_errno_defs.h"
linux-user/user-internals.h:184:#include "target_cpu.h"
linux-user/user-internals.h:185:#include "target_structs.h"

I'll add a paragraph to describe that.

> 
>> +  Such ``target`` suffixed headers usually *taint* the files including them
>> +  by making them target specific.
>> +
>> +  Files common to all targets should use the ``common`` suffix, to provide
>> +  a hint that these files can be safely included from common code.
>> +
>> +
> 
> An additional question that kind of confuses me is whether header file
> naming should use "-" or "_" to connect prefixes/suffixes?

Yeah, we use a mix of both with no particular preference.

Not sure it is worth cleaning only for aesthetic style, let's see
what other think.

> 
>>   Block structure
>>   ===============
>>   
>> -- 
>> 2.41.0
>>

Thanks!

Phil.