diff mbox series

[PATCH-for-6.2?,1/3] docs/devel/style: Improve GLib functions rST rendering

Message ID 20211116151317.2691125-2-philmd@redhat.com (mailing list archive)
State New, archived
Headers show
Series docs/devel/style: Improve rST rendering | expand

Commit Message

Philippe Mathieu-Daudé Nov. 16, 2021, 3:13 p.m. UTC
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 docs/devel/style.rst | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

Comments

Darren Kenny Nov. 18, 2021, 10:58 a.m. UTC | #1
Hi Philippe,

There are some inconsistencies in the use of '()' when referring to
functions or macros below...

On Tuesday, 2021-11-16 at 16:13:15 +01, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  docs/devel/style.rst | 31 ++++++++++++++++---------------
>  1 file changed, 16 insertions(+), 15 deletions(-)
>
> diff --git a/docs/devel/style.rst b/docs/devel/style.rst
> index 260e3263fa0..415a6b9d700 100644
> --- a/docs/devel/style.rst
> +++ b/docs/devel/style.rst
> @@ -413,13 +413,14 @@ multiple exist paths you can also improve the readability of the code
>  by using ``g_autofree`` and related annotations. See :ref:`autofree-ref`
>  for more details.
>  
> -Calling ``g_malloc`` with a zero size is valid and will return NULL.
> +Calling ``g_malloc`` with a zero size is valid and will return ``NULL``.
>

g_malloc() ?

>  
>  Prefer ``g_new(T, n)`` instead of ``g_malloc(sizeof(T) * n)`` for the following
>  reasons:
>  
> -* It catches multiplication overflowing size_t;
> -* It returns T ``*`` instead of void ``*``, letting compiler catch more type errors.
> +* It catches multiplication overflowing ``size_t``;
> +* It returns ``T *`` instead of ``void *``, letting compiler catch more type
> +  errors.
>  
>  Declarations like
>  
> @@ -444,14 +445,14 @@ use this similar function when possible, but note its different signature:
>  
>      void pstrcpy(char *dest, int dest_buf_size, const char *src)
>  
> -Don't use strcat because it can't check for buffer overflows, but:
> +Don't use ``strcat`` because it can't check for buffer overflows, but:
>

strcat() ?

>  
>  .. code-block:: c
>  
>      char *pstrcat(char *buf, int buf_size, const char *s)
>  
> -The same limitation exists with sprintf and vsprintf, so use snprintf and
> -vsnprintf.
> +The same limitation exists with ``sprintf`` and ``vsprintf``, so use

sprintf() and vsprintf()?

> +``snprintf`` and ``vsnprintf``.
>

snprintf() and vsnprintf()?

>  
>  QEMU provides other useful string functions:
>  
> @@ -464,8 +465,8 @@ QEMU provides other useful string functions:
>  There are also replacement character processing macros for isxyz and toxyz,
>  so instead of e.g. isalnum you should use qemu_isalnum.
>

Should this be isalnum() and qemu_isalnum()?

>  
> -Because of the memory management rules, you must use g_strdup/g_strndup
> -instead of plain strdup/strndup.
> +Because of the memory management rules, you must use ``g_strdup/g_strndup``
>

Wonder should this be ``g_strdup()``/``g_strndup()``

> +instead of plain ``strdup/strndup``.
>

And ``strdup()``/``strndup()``

>  
>  Printf-style functions
>  ======================
> @@ -524,10 +525,10 @@ automatic cleanup:
>  
>  Most notably:
>  
> -* g_autofree - will invoke g_free() on the variable going out of scope
> +* ``g_autofree`` - will invoke ``g_free()`` on the variable going out of scope
>

g_autofree() ?

>  
> -* g_autoptr - for structs / objects, will invoke the cleanup func created
> -  by a previous use of G_DEFINE_AUTOPTR_CLEANUP_FUNC. This is
> +* ``g_autoptr`` - for structs / objects, will invoke the cleanup func created
>

g_autoptr() ?

> +  by a previous use of ``G_DEFINE_AUTOPTR_CLEANUP_FUNC``. This is
>    supported for most GLib data types and GObjects
>  
>  For example, instead of
> @@ -551,7 +552,7 @@ For example, instead of
>          return ret;
>      }
>  
> -Using g_autofree/g_autoptr enables the code to be written as:
> +Using ``g_autofree/g_autoptr`` enables the code to be written as:
>

``g_autofree()``/``g_autoptr()`` ?

>  
>  .. code-block:: c
>  
> @@ -569,13 +570,13 @@ Using g_autofree/g_autoptr enables the code to be written as:
>  While this generally results in simpler, less leak-prone code, there
>  are still some caveats to beware of
>  
> -* Variables declared with g_auto* MUST always be initialized,
> +* Variables declared with ``g_auto*`` MUST always be initialized,
>

g_auto*() ?

>    otherwise the cleanup function will use uninitialized stack memory
>  
> -* If a variable declared with g_auto* holds a value which must
> +* If a variable declared with ``g_auto*`` holds a value which must
>

g_auto*() ?

>    live beyond the life of the function, that value must be saved
>    and the original variable NULL'd out. This can be simpler using
> -  g_steal_pointer
> +  ``g_steal_pointer``
>

g_steal_pointer() ?

Thanks,

Darren.
Philippe Mathieu-Daudé Nov. 18, 2021, 12:12 p.m. UTC | #2
On 11/18/21 11:58, Darren Kenny wrote:
> Hi Philippe,
> 
> There are some inconsistencies in the use of '()' when referring to
> functions or macros below...

Daniel, if you agree with Darren comments I can respin addressing them.

> On Tuesday, 2021-11-16 at 16:13:15 +01, Philippe Mathieu-Daudé wrote:
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>>  docs/devel/style.rst | 31 ++++++++++++++++---------------
>>  1 file changed, 16 insertions(+), 15 deletions(-)
>>
>> diff --git a/docs/devel/style.rst b/docs/devel/style.rst
>> index 260e3263fa0..415a6b9d700 100644
>> --- a/docs/devel/style.rst
>> +++ b/docs/devel/style.rst
>> @@ -413,13 +413,14 @@ multiple exist paths you can also improve the readability of the code
>>  by using ``g_autofree`` and related annotations. See :ref:`autofree-ref`
>>  for more details.
>>  
>> -Calling ``g_malloc`` with a zero size is valid and will return NULL.
>> +Calling ``g_malloc`` with a zero size is valid and will return ``NULL``.
>>
> 
> g_malloc() ?
> 
>>  
>>  Prefer ``g_new(T, n)`` instead of ``g_malloc(sizeof(T) * n)`` for the following
>>  reasons:
>>  
>> -* It catches multiplication overflowing size_t;
>> -* It returns T ``*`` instead of void ``*``, letting compiler catch more type errors.
>> +* It catches multiplication overflowing ``size_t``;
>> +* It returns ``T *`` instead of ``void *``, letting compiler catch more type
>> +  errors.
>>  
>>  Declarations like
>>  
>> @@ -444,14 +445,14 @@ use this similar function when possible, but note its different signature:
>>  
>>      void pstrcpy(char *dest, int dest_buf_size, const char *src)
>>  
>> -Don't use strcat because it can't check for buffer overflows, but:
>> +Don't use ``strcat`` because it can't check for buffer overflows, but:
>>
> 
> strcat() ?
> 
>>  
>>  .. code-block:: c
>>  
>>      char *pstrcat(char *buf, int buf_size, const char *s)
>>  
>> -The same limitation exists with sprintf and vsprintf, so use snprintf and
>> -vsnprintf.
>> +The same limitation exists with ``sprintf`` and ``vsprintf``, so use
> 
> sprintf() and vsprintf()?
> 
>> +``snprintf`` and ``vsnprintf``.
>>
> 
> snprintf() and vsnprintf()?
> 
>>  
>>  QEMU provides other useful string functions:
>>  
>> @@ -464,8 +465,8 @@ QEMU provides other useful string functions:
>>  There are also replacement character processing macros for isxyz and toxyz,
>>  so instead of e.g. isalnum you should use qemu_isalnum.
>>
> 
> Should this be isalnum() and qemu_isalnum()?
> 
>>  
>> -Because of the memory management rules, you must use g_strdup/g_strndup
>> -instead of plain strdup/strndup.
>> +Because of the memory management rules, you must use ``g_strdup/g_strndup``
>>
> 
> Wonder should this be ``g_strdup()``/``g_strndup()``
> 
>> +instead of plain ``strdup/strndup``.
>>
> 
> And ``strdup()``/``strndup()``
> 
>>  
>>  Printf-style functions
>>  ======================
>> @@ -524,10 +525,10 @@ automatic cleanup:
>>  
>>  Most notably:
>>  
>> -* g_autofree - will invoke g_free() on the variable going out of scope
>> +* ``g_autofree`` - will invoke ``g_free()`` on the variable going out of scope
>>
> 
> g_autofree() ?
> 
>>  
>> -* g_autoptr - for structs / objects, will invoke the cleanup func created
>> -  by a previous use of G_DEFINE_AUTOPTR_CLEANUP_FUNC. This is
>> +* ``g_autoptr`` - for structs / objects, will invoke the cleanup func created
>>
> 
> g_autoptr() ?
> 
>> +  by a previous use of ``G_DEFINE_AUTOPTR_CLEANUP_FUNC``. This is
>>    supported for most GLib data types and GObjects
>>  
>>  For example, instead of
>> @@ -551,7 +552,7 @@ For example, instead of
>>          return ret;
>>      }
>>  
>> -Using g_autofree/g_autoptr enables the code to be written as:
>> +Using ``g_autofree/g_autoptr`` enables the code to be written as:
>>
> 
> ``g_autofree()``/``g_autoptr()`` ?
> 
>>  
>>  .. code-block:: c
>>  
>> @@ -569,13 +570,13 @@ Using g_autofree/g_autoptr enables the code to be written as:
>>  While this generally results in simpler, less leak-prone code, there
>>  are still some caveats to beware of
>>  
>> -* Variables declared with g_auto* MUST always be initialized,
>> +* Variables declared with ``g_auto*`` MUST always be initialized,
>>
> 
> g_auto*() ?
> 
>>    otherwise the cleanup function will use uninitialized stack memory
>>  
>> -* If a variable declared with g_auto* holds a value which must
>> +* If a variable declared with ``g_auto*`` holds a value which must
>>
> 
> g_auto*() ?
> 
>>    live beyond the life of the function, that value must be saved
>>    and the original variable NULL'd out. This can be simpler using
>> -  g_steal_pointer
>> +  ``g_steal_pointer``
>>
> 
> g_steal_pointer() ?
> 
> Thanks,
> 
> Darren.
>
Daniel P. Berrangé Nov. 18, 2021, 1:03 p.m. UTC | #3
On Thu, Nov 18, 2021 at 01:12:26PM +0100, Philippe Mathieu-Daudé wrote:
> On 11/18/21 11:58, Darren Kenny wrote:
> > Hi Philippe,
> > 
> > There are some inconsistencies in the use of '()' when referring to
> > functions or macros below...
> 
> Daniel, if you agree with Darren comments I can respin addressing them.

It is fine with me.


Regards,
Daniel
diff mbox series

Patch

diff --git a/docs/devel/style.rst b/docs/devel/style.rst
index 260e3263fa0..415a6b9d700 100644
--- a/docs/devel/style.rst
+++ b/docs/devel/style.rst
@@ -413,13 +413,14 @@  multiple exist paths you can also improve the readability of the code
 by using ``g_autofree`` and related annotations. See :ref:`autofree-ref`
 for more details.
 
-Calling ``g_malloc`` with a zero size is valid and will return NULL.
+Calling ``g_malloc`` with a zero size is valid and will return ``NULL``.
 
 Prefer ``g_new(T, n)`` instead of ``g_malloc(sizeof(T) * n)`` for the following
 reasons:
 
-* It catches multiplication overflowing size_t;
-* It returns T ``*`` instead of void ``*``, letting compiler catch more type errors.
+* It catches multiplication overflowing ``size_t``;
+* It returns ``T *`` instead of ``void *``, letting compiler catch more type
+  errors.
 
 Declarations like
 
@@ -444,14 +445,14 @@  use this similar function when possible, but note its different signature:
 
     void pstrcpy(char *dest, int dest_buf_size, const char *src)
 
-Don't use strcat because it can't check for buffer overflows, but:
+Don't use ``strcat`` because it can't check for buffer overflows, but:
 
 .. code-block:: c
 
     char *pstrcat(char *buf, int buf_size, const char *s)
 
-The same limitation exists with sprintf and vsprintf, so use snprintf and
-vsnprintf.
+The same limitation exists with ``sprintf`` and ``vsprintf``, so use
+``snprintf`` and ``vsnprintf``.
 
 QEMU provides other useful string functions:
 
@@ -464,8 +465,8 @@  QEMU provides other useful string functions:
 There are also replacement character processing macros for isxyz and toxyz,
 so instead of e.g. isalnum you should use qemu_isalnum.
 
-Because of the memory management rules, you must use g_strdup/g_strndup
-instead of plain strdup/strndup.
+Because of the memory management rules, you must use ``g_strdup/g_strndup``
+instead of plain ``strdup/strndup``.
 
 Printf-style functions
 ======================
@@ -524,10 +525,10 @@  automatic cleanup:
 
 Most notably:
 
-* g_autofree - will invoke g_free() on the variable going out of scope
+* ``g_autofree`` - will invoke ``g_free()`` on the variable going out of scope
 
-* g_autoptr - for structs / objects, will invoke the cleanup func created
-  by a previous use of G_DEFINE_AUTOPTR_CLEANUP_FUNC. This is
+* ``g_autoptr`` - for structs / objects, will invoke the cleanup func created
+  by a previous use of ``G_DEFINE_AUTOPTR_CLEANUP_FUNC``. This is
   supported for most GLib data types and GObjects
 
 For example, instead of
@@ -551,7 +552,7 @@  For example, instead of
         return ret;
     }
 
-Using g_autofree/g_autoptr enables the code to be written as:
+Using ``g_autofree/g_autoptr`` enables the code to be written as:
 
 .. code-block:: c
 
@@ -569,13 +570,13 @@  Using g_autofree/g_autoptr enables the code to be written as:
 While this generally results in simpler, less leak-prone code, there
 are still some caveats to beware of
 
-* Variables declared with g_auto* MUST always be initialized,
+* Variables declared with ``g_auto*`` MUST always be initialized,
   otherwise the cleanup function will use uninitialized stack memory
 
-* If a variable declared with g_auto* holds a value which must
+* If a variable declared with ``g_auto*`` holds a value which must
   live beyond the life of the function, that value must be saved
   and the original variable NULL'd out. This can be simpler using
-  g_steal_pointer
+  ``g_steal_pointer``
 
 
 .. code-block:: c