[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH-for-6.2? 1/3] docs/devel/style: Improve GLib functions rST re
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [PATCH-for-6.2? 1/3] docs/devel/style: Improve GLib functions rST rendering |
Date: |
Thu, 18 Nov 2021 13:12:26 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.2.0 |
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.
>