[RFC PATCH] CODING_STYLE.rst: flesh out our naming conventions.

Alex Bennée posted 1 patch 3 years, 9 months ago
Test docker-quick@centos7 failed
Test docker-mingw@fedora failed
Test checkpatch failed
Test FreeBSD failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200806114036.10646-1-alex.bennee@linaro.org
There is a newer version of this series
CODING_STYLE.rst | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
[RFC PATCH] CODING_STYLE.rst: flesh out our naming conventions.
Posted by Alex Bennée 3 years, 9 months ago
Mention a few of the more common naming conventions we follow in the
code base including common variable names and function prefix and
suffix examples.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 CODING_STYLE.rst | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/CODING_STYLE.rst b/CODING_STYLE.rst
index 427699e0e42..17cb42ba761 100644
--- a/CODING_STYLE.rst
+++ b/CODING_STYLE.rst
@@ -109,9 +109,32 @@ names are lower_case_with_underscores_ending_with_a_t, like the POSIX
 uint64_t and family.  Note that this last convention contradicts POSIX
 and is therefore likely to be changed.
 
+Variable Naming Conventions
+---------------------------
+
+A number of common short naming conventions exist for variables that use
+common QEMU types. For example when dealing with the architecture
+independent CPUState this is often in a ``cs`` pointer variable
+whereas the concrete CPUArchState us usually held in a pointer called
+``env``.
+
+Likewise in device emulation code the common DeviceState is usually
+called ``dev`` with the actual state structure often the very terse
+``s`` or maybe ``foodev``.
+
+Function Naming Conventions
+---------------------------
+
 When wrapping standard library functions, use the prefix ``qemu_`` to alert
 readers that they are seeing a wrapped version; otherwise avoid this prefix.
 
+Functions that are expected to be called with some sort of lock held
+usually have the suffix ``_locked``.
+
+Public functions (i.e. declared in public headers) tend to be prefixes
+with the subsystem or file they came from. For example ``tlb_`` for
+functions from ``cputlb.c`` or ``cpu_`` for functions from cpus.c.
+
 Block structure
 ===============
 
-- 
2.20.1


Re: [RFC PATCH] CODING_STYLE.rst: flesh out our naming conventions.
Posted by Cornelia Huck 3 years, 9 months ago
On Thu,  6 Aug 2020 12:40:36 +0100
Alex Bennée <alex.bennee@linaro.org> wrote:

> Mention a few of the more common naming conventions we follow in the
> code base including common variable names and function prefix and
> suffix examples.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  CODING_STYLE.rst | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/CODING_STYLE.rst b/CODING_STYLE.rst
> index 427699e0e42..17cb42ba761 100644
> --- a/CODING_STYLE.rst
> +++ b/CODING_STYLE.rst
> @@ -109,9 +109,32 @@ names are lower_case_with_underscores_ending_with_a_t, like the POSIX
>  uint64_t and family.  Note that this last convention contradicts POSIX
>  and is therefore likely to be changed.
>  
> +Variable Naming Conventions
> +---------------------------
> +
> +A number of common short naming conventions exist for variables that use
> +common QEMU types. For example when dealing with the architecture

s/example/example,/

> +independent CPUState this is often in a ``cs`` pointer variable

"the architecture independent CPUState is often held in a ``cs``
pointer variable," ?

> +whereas the concrete CPUArchState us usually held in a pointer called
> +``env``.
> +
> +Likewise in device emulation code the common DeviceState is usually

s/Likewise/Likewise,/

> +called ``dev`` with the actual state structure often the very terse

"while the actual statue structure often uses..." ?

> +``s`` or maybe ``foodev``.
> +
> +Function Naming Conventions
> +---------------------------
> +
>  When wrapping standard library functions, use the prefix ``qemu_`` to alert
>  readers that they are seeing a wrapped version; otherwise avoid this prefix.
>  
> +Functions that are expected to be called with some sort of lock held
> +usually have the suffix ``_locked``.

Hm, I think this is mostly for those cases where you have two versions?
What about:

"If there are two versions of a function to be called with or without a
lock held, the function that expects the lock to be already usually
uses the suffix ``_locked``."

> +
> +Public functions (i.e. declared in public headers) tend to be prefixes

s/prefixes/prefixed/

> +with the subsystem or file they came from. For example ``tlb_`` for

s/example/example,/

> +functions from ``cputlb.c`` or ``cpu_`` for functions from cpus.c.
> +
>  Block structure
>  ===============
>  


Re: [RFC PATCH] CODING_STYLE.rst: flesh out our naming conventions.
Posted by Claudio Fontana 3 years, 9 months ago
On 8/6/20 1:40 PM, Alex Bennée wrote:
> Mention a few of the more common naming conventions we follow in the
> code base including common variable names and function prefix and
> suffix examples.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  CODING_STYLE.rst | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/CODING_STYLE.rst b/CODING_STYLE.rst
> index 427699e0e42..17cb42ba761 100644
> --- a/CODING_STYLE.rst
> +++ b/CODING_STYLE.rst
> @@ -109,9 +109,32 @@ names are lower_case_with_underscores_ending_with_a_t, like the POSIX
>  uint64_t and family.  Note that this last convention contradicts POSIX
>  and is therefore likely to be changed.
>  
> +Variable Naming Conventions
> +---------------------------
> +
> +A number of common short naming conventions exist for variables that use
> +common QEMU types. For example when dealing with the architecture
> +independent CPUState this is often in a ``cs`` pointer variable
> +whereas the concrete CPUArchState us usually held in a pointer called
> +``env``.
> +
> +Likewise in device emulation code the common DeviceState is usually
> +called ``dev`` with the actual state structure often the very terse
> +``s`` or maybe ``foodev``.
> +
> +Function Naming Conventions
> +---------------------------
> +
>  When wrapping standard library functions, use the prefix ``qemu_`` to alert
>  readers that they are seeing a wrapped version; otherwise avoid this prefix.
>  
> +Functions that are expected to be called with some sort of lock held
> +usually have the suffix ``_locked``.
> +
> +Public functions (i.e. declared in public headers) tend to be prefixes
> +with the subsystem or file they came from. For example ``tlb_`` for
> +functions from ``cputlb.c`` or ``cpu_`` for functions from cpus.c.

Hi Alex,

agree with the sentiment, for cpus.c I would suggest cpus_ though.

There are otherwise so many cpu_ functions and cpu.c modules.. using cpus_ would help a lot to understand that it is part of softmmu/cpus.c

Wdyt?

Also cpus.c contains a lot of qemu_ that is not used to wrap standard library functions.

After the current cpus refactoring, I could take care of improving the naming conventions in cpus.c

Thanks,

Claudio


> +
>  Block structure
>  ===============
>  
> 


Re: [RFC PATCH] CODING_STYLE.rst: flesh out our naming conventions.
Posted by Alex Bennée 3 years, 9 months ago
Claudio Fontana <cfontana@suse.de> writes:

> On 8/6/20 1:40 PM, Alex Bennée wrote:
>> Mention a few of the more common naming conventions we follow in the
>> code base including common variable names and function prefix and
>> suffix examples.
>> 
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> ---
>>  CODING_STYLE.rst | 23 +++++++++++++++++++++++
>>  1 file changed, 23 insertions(+)
>> 
>> diff --git a/CODING_STYLE.rst b/CODING_STYLE.rst
>> index 427699e0e42..17cb42ba761 100644
>> --- a/CODING_STYLE.rst
>> +++ b/CODING_STYLE.rst
>> @@ -109,9 +109,32 @@ names are lower_case_with_underscores_ending_with_a_t, like the POSIX
>>  uint64_t and family.  Note that this last convention contradicts POSIX
>>  and is therefore likely to be changed.
>>  
>> +Variable Naming Conventions
>> +---------------------------
>> +
>> +A number of common short naming conventions exist for variables that use
>> +common QEMU types. For example when dealing with the architecture
>> +independent CPUState this is often in a ``cs`` pointer variable
>> +whereas the concrete CPUArchState us usually held in a pointer called
>> +``env``.
>> +
>> +Likewise in device emulation code the common DeviceState is usually
>> +called ``dev`` with the actual state structure often the very terse
>> +``s`` or maybe ``foodev``.
>> +
>> +Function Naming Conventions
>> +---------------------------
>> +
>>  When wrapping standard library functions, use the prefix ``qemu_`` to alert
>>  readers that they are seeing a wrapped version; otherwise avoid this prefix.
>>  
>> +Functions that are expected to be called with some sort of lock held
>> +usually have the suffix ``_locked``.
>> +
>> +Public functions (i.e. declared in public headers) tend to be prefixes
>> +with the subsystem or file they came from. For example ``tlb_`` for
>> +functions from ``cputlb.c`` or ``cpu_`` for functions from cpus.c.
>
> Hi Alex,
>
> agree with the sentiment, for cpus.c I would suggest cpus_ though.
>
> There are otherwise so many cpu_ functions and cpu.c modules.. using
> cpus_ would help a lot to understand that it is part of softmmu/cpus.c
>
> Wdyt?

I was mainly documenting existing conventions - I guess cpus is a poor
example to follow as it has so many cpu_ functions in them. I guess the
prefix is a loose association at best.

> Also cpus.c contains a lot of qemu_ that is not used to wrap standard
> library functions.

Certainly a lot of the static internal functions could be shorted names
dropping the qemu_ prefix. Most uses of qemu_ across the code base are
utility functions although not all wrap existing functions.

> After the current cpus refactoring, I could take care of improving the
> naming conventions in cpus.c

I wouldn't expend a lot of effort on a grand re-naming but certainly
it's worth cleaning stuff up as you go moving things about.

>
> Thanks,
>
> Claudio
>
>
>> +
>>  Block structure
>>  ===============
>>  
>> 


-- 
Alex Bennée