All of lore.kernel.org
 help / color / mirror / Atom feed
From: Darren Kenny <darren.kenny@oracle.com>
To: "Philippe Mathieu-Daudé" <philmd@redhat.com>, qemu-devel@nongnu.org
Cc: "Peter Maydell" <peter.maydell@linaro.org>,
	"Daniel P . Berrange" <berrange@redhat.com>,
	"Markus Armbruster" <armbru@redhat.com>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>
Subject: Re: [PATCH-for-6.2? 3/3] docs/devel/style: Improve types/qualifiers rST rendering
Date: Thu, 18 Nov 2021 11:04:21 +0000	[thread overview]
Message-ID: <m2ee7dn2ru.fsf@oracle.com> (raw)
In-Reply-To: <20211116151317.2691125-4-philmd@redhat.com>

Hi Philippe,

A couple here too w.r.t. function/macros...

On Tuesday, 2021-11-16 at 16:13:17 +01, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  docs/devel/style.rst | 111 ++++++++++++++++++++++---------------------
>  1 file changed, 56 insertions(+), 55 deletions(-)
>
> diff --git a/docs/devel/style.rst b/docs/devel/style.rst
> index 21f0f213193..f9f063ed8cb 100644
> --- a/docs/devel/style.rst
> +++ b/docs/devel/style.rst
> @@ -111,7 +111,7 @@ Variables are lower_case_with_underscores; easy to type and read.  Structured
>  type names are in CamelCase; harder to type but standing out.  Enum type
>  names and function type names should also be in CamelCase.  Scalar type
>  names are lower_case_with_underscores_ending_with_a_t, like the POSIX
> -uint64_t and family.  Note that this last convention contradicts POSIX
> +``uint64_t`` and family.  Note that this last convention contradicts POSIX
>  and is therefore likely to be changed.
>  
>  Variable Naming Conventions
> @@ -195,9 +195,9 @@ blocks) are generally not allowed; declarations should be at the beginning
>  of blocks.
>  
>  Every now and then, an exception is made for declarations inside a
> -#ifdef or #ifndef block: if the code looks nicer, such declarations can
> +``#ifdef`` or ``#ifndef`` block: if the code looks nicer, such declarations can
>  be placed at the top of the block even if there are statements above.
> -On the other hand, however, it's often best to move that #ifdef/#ifndef
> +On the other hand, however, it's often best to move that ``#ifdef/#ifndef``
>  block to a separate function altogether.
>  
>  Conditional statements
> @@ -220,13 +220,13 @@ even when the constant is on the right.
>  Comment style
>  =============
>  
> -We use traditional C-style /``*`` ``*``/ comments and avoid // comments.
> +We use traditional C-style ``/*`` ``*/`` comments and avoid ``//`` comments.
>  
> -Rationale: The // form is valid in C99, so this is purely a matter of
> +Rationale: The ``//`` form is valid in C99, so this is purely a matter of
>  consistency of style. The checkpatch script will warn you about this.
>  
>  Multiline comment blocks should have a row of stars on the left,
> -and the initial /``*`` and terminating ``*``/ both on their own lines:
> +and the initial ``/*`` and terminating ``*/`` both on their own lines:
>  
>  .. code-block:: c
>  
> @@ -290,57 +290,57 @@ a few useful guidelines here.
>  Scalars
>  -------
>  
> -If you're using "int" or "long", odds are good that there's a better type.
> -If a variable is counting something, it should be declared with an
> -unsigned type.
> +If you're using '``int``' or '``long``', odds are good that there's a better
> +type.  If a variable is counting something, it should be declared with an
> +*unsigned* type.
>  
> -If it's host memory-size related, size_t should be a good choice (use
> -ssize_t only if required). Guest RAM memory offsets must use ram_addr_t,
> +If it's host memory-size related, ``size_t`` should be a good choice (use
> +``ssize_t`` only if required). Guest RAM memory offsets must use ``ram_addr_t``,
>  but only for RAM, it may not cover whole guest address space.
>  
> -If it's file-size related, use off_t.
> -If it's file-offset related (i.e., signed), use off_t.
> -If it's just counting small numbers use "unsigned int";
> +If it's file-size related, use ``off_t``.
> +If it's file-offset related (i.e., signed), use ``off_t``.
> +If it's just counting small numbers use '``unsigned int``';
>  (on all but oddball embedded systems, you can assume that that
>  type is at least four bytes wide).
>  
>  In the event that you require a specific width, use a standard type
> -like int32_t, uint32_t, uint64_t, etc.  The specific types are
> +like ``int32_t``, ``uint32_t``, ``uint64_t``, etc.  The specific types are
>  mandatory for VMState fields.
>  
> -Don't use Linux kernel internal types like u32, __u32 or __le32.
> +Don't use Linux kernel internal types like ``u32``, ``__u32`` or ``__le32``.
>  
> -Use hwaddr for guest physical addresses except pcibus_t
> -for PCI addresses.  In addition, ram_addr_t is a QEMU internal address
> +Use ``hwaddr`` for guest physical addresses except ``pcibus_t``
> +for PCI addresses.  In addition, ``ram_addr_t`` is a QEMU internal address
>  space that maps guest RAM physical addresses into an intermediate
>  address space that can map to host virtual address spaces.  Generally
> -speaking, the size of guest memory can always fit into ram_addr_t but
> +speaking, the size of guest memory can always fit into ``ram_addr_t`` but
>  it would not be correct to store an actual guest physical address in a
> -ram_addr_t.
> +``ram_addr_t``.
>  
>  For CPU virtual addresses there are several possible types.
> -vaddr is the best type to use to hold a CPU virtual address in
> +``vaddr`` is the best type to use to hold a CPU virtual address in
>  target-independent code. It is guaranteed to be large enough to hold a
>  virtual address for any target, and it does not change size from target
>  to target. It is always unsigned.
> -target_ulong is a type the size of a virtual address on the CPU; this means
> +``target_ulong`` is a type the size of a virtual address on the CPU; this means
>  it may be 32 or 64 bits depending on which target is being built. It should
>  therefore be used only in target-specific code, and in some
>  performance-critical built-per-target core code such as the TLB code.
> -There is also a signed version, target_long.
> -abi_ulong is for the ``*``-user targets, and represents a type the size of
> -'void ``*``' in that target's ABI. (This may not be the same as the size of a
> +There is also a signed version, ``target_long``.
> +``abi_ulong`` is for the ``*-user`` targets, and represents a type the size of
> +'``void *``' in that target's ABI. (This may not be the same as the size of a
>  full CPU virtual address in the case of target ABIs which use 32 bit pointers
> -on 64 bit CPUs, like sparc32plus.) Definitions of structures that must match
> +on 64 bit CPUs, like *sparc32plus*.) Definitions of structures that must match
>  the target's ABI must use this type for anything that on the target is defined
> -to be an 'unsigned long' or a pointer type.
> -There is also a signed version, abi_long.
> +to be an '``unsigned long``' or a pointer type.
> +There is also a signed version, ``abi_long``.
>  
>  Of course, take all of the above with a grain of salt.  If you're about
> -to use some system interface that requires a type like size_t, pid_t or
> -off_t, use matching types for any corresponding variables.
> +to use some system interface that requires a type like ``size_t``, ``pid_t`` or
> +``off_t``, use matching types for any corresponding variables.
>  
> -Also, if you try to use e.g., "unsigned int" as a type, and that
> +Also, if you try to use e.g., '``unsigned int``' as a type, and that
>  conflicts with the signedness of a related variable, sometimes
>  it's best just to use the *wrong* type, if "pulling the thread"
>  and fixing all related variables would be too invasive.
> @@ -352,9 +352,9 @@ casts, then reconsider or ask for help.
>  Pointers
>  --------
>  
> -Ensure that all of your pointers are "const-correct".
> +Ensure that all of your pointers are "``const``-correct".
>  Unless a pointer is used to modify the pointed-to storage,
> -give it the "const" attribute.  That way, the reader knows
> +give it the '``const``' attribute.  That way, the reader knows
>  up-front that this is a read-only pointer.  Perhaps more
>  importantly, if we're diligent about this, when you see a non-const
>  pointer, you're guaranteed that it is used to modify the storage
> @@ -363,7 +363,7 @@ it points to, or it is aliased to another pointer that is.
>  Typedefs
>  --------
>  
> -Typedefs are used to eliminate the redundant 'struct' keyword, since type
> +Typedefs are used to eliminate the redundant '``struct``' keyword, since type
>  names have a different style than other identifiers ("CamelCase" versus
>  "snake_case").  Each named struct type should have a CamelCase name and a
>  corresponding typedef.
> @@ -462,8 +462,8 @@ QEMU provides other useful string functions:
>      int stristart(const char *str, const char *val, const char **ptr)
>      int qemu_strnlen(const char *s, int max_len)
>  
> -There are also replacement character processing macros for isxyz and toxyz,
> -so instead of e.g. isalnum you should use qemu_isalnum.
> +There are also replacement character processing macros for ``isxyz`` and
> +``toxyz``, so instead of e.g. ``isalnum`` you should use ``qemu_isalnum``.
>

(Looks like a repeat of a change in patch 1, but possibly a different location)

isalnum() and qemu_isalnum()?

Thanks,

Darren.


      reply	other threads:[~2021-11-18 11:07 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-16 15:13 [PATCH-for-6.2? 0/3] docs/devel/style: Improve rST rendering Philippe Mathieu-Daudé
2021-11-16 15:13 ` [PATCH-for-6.2? 1/3] docs/devel/style: Improve GLib functions " Philippe Mathieu-Daudé
2021-11-18 10:58   ` Darren Kenny
2021-11-18 12:12     ` Philippe Mathieu-Daudé
2021-11-18 13:03       ` Daniel P. Berrangé
2021-11-16 15:13 ` [PATCH-for-6.2? 2/3] docs/devel/style: Improve Error** " Philippe Mathieu-Daudé
2021-11-16 15:13 ` [PATCH-for-6.2? 3/3] docs/devel/style: Improve types/qualifiers " Philippe Mathieu-Daudé
2021-11-18 11:04   ` Darren Kenny [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=m2ee7dn2ru.fsf@oracle.com \
    --to=darren.kenny@oracle.com \
    --cc=armbru@redhat.com \
    --cc=berrange@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=philmd@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.