diff mbox series

[9/9] docs/style: call out the use of GUARD macros

Message ID 20230420155723.1711048-10-alex.bennee@linaro.org
State New
Headers show
Series docs: various (style, punctuation and typo fixes) | expand

Commit Message

Alex Bennée April 20, 2023, 3:57 p.m. UTC
There use makes our code safer so we should mention them.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 docs/devel/style.rst | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

Comments

Juan Quintela April 20, 2023, 8:58 p.m. UTC | #1
Alex Bennée <alex.bennee@linaro.org> wrote:
> There use makes our code safer so we should mention them.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>


Reviewed-by: Juan Quintela <quintela@redhat.com>
Philippe Mathieu-Daudé April 21, 2023, 6:17 a.m. UTC | #2
On 20/4/23 17:57, Alex Bennée wrote:
> There use makes our code safer so we should mention them.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

> ---
>   docs/devel/style.rst | 36 ++++++++++++++++++++++++++++++++++++
>   1 file changed, 36 insertions(+)
> 
> diff --git a/docs/devel/style.rst b/docs/devel/style.rst
> index 0bd01f3fca..b50a981a86 100644
> --- a/docs/devel/style.rst
> +++ b/docs/devel/style.rst
> @@ -657,6 +657,42 @@ that declaration and the new code.
>   
>   See :ref:`qom` for more details.
>   
> +QEMU GUARD macros
> +=================
> +
> +QEMU provides a number of ``_GUARD`` macros intended to make the
> +handling of multiple exit paths easier. For example using
> +``QEMU_LOCK_GUARD`` to take a lock will ensure the lock is released on
> +exit from the function.
> +
> +.. code-block:: c
> +
> +    static int my_critical_function(SomeState *s, void *data)
> +    {
> +        QEMU_LOCK_GUARD(&s->lock);
> +        do_thing1(data);
> +        if (check_state2(data)) {
> +            return -1;
> +        }
> +        do_thing3(data);
> +        return 0;
> +    }
> +
> +will ensure s->lock is released however the function is exited. There
> +are often ``WITH_`` forms of macros which more easily wrap around a
> +block inside a function.
> +
> +.. code-block:: c
> +
> +    WITH_RCU_READ_LOCK_GUARD() {
> +        QTAILQ_FOREACH_RCU(kid, &bus->children, sibling) {
> +            err = do_the_thing(kid->child);
> +            if (err < 0) {
> +                return err;
> +            }
> +        }
> +    }
> +
>   Error handling and reporting
>   ============================
>
Vladimir Sementsov-Ogievskiy April 21, 2023, 9:07 a.m. UTC | #3
On 20.04.23 18:57, Alex Bennée wrote:
> There use makes our code safer so we should mention them.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>


> ---
>   docs/devel/style.rst | 36 ++++++++++++++++++++++++++++++++++++
>   1 file changed, 36 insertions(+)
> 
> diff --git a/docs/devel/style.rst b/docs/devel/style.rst
> index 0bd01f3fca..b50a981a86 100644
> --- a/docs/devel/style.rst
> +++ b/docs/devel/style.rst
> @@ -657,6 +657,42 @@ that declaration and the new code.
>   
>   See :ref:`qom` for more details.
>   
> +QEMU GUARD macros
> +=================
> +
> +QEMU provides a number of ``_GUARD`` macros intended to make the
> +handling of multiple exit paths easier. For example using
> +``QEMU_LOCK_GUARD`` to take a lock will ensure the lock is released on
> +exit from the function.
> +
> +.. code-block:: c
> +
> +    static int my_critical_function(SomeState *s, void *data)
> +    {
> +        QEMU_LOCK_GUARD(&s->lock);
> +        do_thing1(data);
> +        if (check_state2(data)) {
> +            return -1;
> +        }
> +        do_thing3(data);
> +        return 0;
> +    }

For more clearness, maybe add an equivalent code with qemu_mutex_lock() / qemu_mutex_unlock(), I mean:

The equivalent code without _GUARD macro makes us to carefully put qemu_mutex_unlock() on all exit points:

.. code-block:: c

     static int my_critical_function(SomeState *s, void *data)
     {
         qemu_mutex_lock(&s->lock);
         do_thing1(data);
         if (check_state2(data)) {
             qemu_mutex_unlock(&s->lock);
             return -1;
         }
         do_thing3(data);
         qemu_mutex_unlock(&s->lock);
         return 0;
     }

> +
> +will ensure s->lock is released however the function is exited. There
> +are often ``WITH_`` forms of macros which more easily wrap around a
> +block inside a function.
> +
> +.. code-block:: c
> +
> +    WITH_RCU_READ_LOCK_GUARD() {
> +        QTAILQ_FOREACH_RCU(kid, &bus->children, sibling) {
> +            err = do_the_thing(kid->child);
> +            if (err < 0) {
> +                return err;
> +            }
> +        }
> +    }
> +

and maybe similar here.

>   Error handling and reporting
>   ============================
>
Alex Bennée April 24, 2023, 9:07 a.m. UTC | #4
Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:

> On 20.04.23 18:57, Alex Bennée wrote:
>> There use makes our code safer so we should mention them.
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>
>
>> ---
>>   docs/devel/style.rst | 36 ++++++++++++++++++++++++++++++++++++
>>   1 file changed, 36 insertions(+)
>> diff --git a/docs/devel/style.rst b/docs/devel/style.rst
>> index 0bd01f3fca..b50a981a86 100644
>> --- a/docs/devel/style.rst
>> +++ b/docs/devel/style.rst
>> @@ -657,6 +657,42 @@ that declaration and the new code.
>>     See :ref:`qom` for more details.
>>   +QEMU GUARD macros
>> +=================
>> +
>> +QEMU provides a number of ``_GUARD`` macros intended to make the
>> +handling of multiple exit paths easier. For example using
>> +``QEMU_LOCK_GUARD`` to take a lock will ensure the lock is released on
>> +exit from the function.
>> +
>> +.. code-block:: c
>> +
>> +    static int my_critical_function(SomeState *s, void *data)
>> +    {
>> +        QEMU_LOCK_GUARD(&s->lock);
>> +        do_thing1(data);
>> +        if (check_state2(data)) {
>> +            return -1;
>> +        }
>> +        do_thing3(data);
>> +        return 0;
>> +    }
>
> For more clearness, maybe add an equivalent code with qemu_mutex_lock() / qemu_mutex_unlock(), I mean:
>
> The equivalent code without _GUARD macro makes us to carefully put qemu_mutex_unlock() on all exit points:
>
> .. code-block:: c
>
>     static int my_critical_function(SomeState *s, void *data)
>     {
>         qemu_mutex_lock(&s->lock);
>         do_thing1(data);
>         if (check_state2(data)) {
>             qemu_mutex_unlock(&s->lock);
>             return -1;
>         }
>         do_thing3(data);
>         qemu_mutex_unlock(&s->lock);
>         return 0;
>     }
>
>> +
>> +will ensure s->lock is released however the function is exited. There
>> +are often ``WITH_`` forms of macros which more easily wrap around a
>> +block inside a function.
>> +
>> +.. code-block:: c
>> +
>> +    WITH_RCU_READ_LOCK_GUARD() {
>> +        QTAILQ_FOREACH_RCU(kid, &bus->children, sibling) {
>> +            err = do_the_thing(kid->child);
>> +            if (err < 0) {
>> +                return err;
>> +            }
>> +        }
>> +    }
>> +
>
> and maybe similar here.

I added the example although I didn't repeat it for the WITH form
because readers should hopefully have understood the idea with the first
example.


>
>>   Error handling and reporting
>>   ============================
>>
Vladimir Sementsov-Ogievskiy April 24, 2023, 12:58 p.m. UTC | #5
On 24.04.23 12:07, Alex Bennée wrote:
> 
> Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:
> 
>> On 20.04.23 18:57, Alex Bennée wrote:
>>> There use makes our code safer so we should mention them.
>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>
>> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>>
>>
>>> ---
>>>    docs/devel/style.rst | 36 ++++++++++++++++++++++++++++++++++++
>>>    1 file changed, 36 insertions(+)
>>> diff --git a/docs/devel/style.rst b/docs/devel/style.rst
>>> index 0bd01f3fca..b50a981a86 100644
>>> --- a/docs/devel/style.rst
>>> +++ b/docs/devel/style.rst
>>> @@ -657,6 +657,42 @@ that declaration and the new code.
>>>      See :ref:`qom` for more details.
>>>    +QEMU GUARD macros
>>> +=================
>>> +
>>> +QEMU provides a number of ``_GUARD`` macros intended to make the
>>> +handling of multiple exit paths easier. For example using
>>> +``QEMU_LOCK_GUARD`` to take a lock will ensure the lock is released on
>>> +exit from the function.
>>> +
>>> +.. code-block:: c
>>> +
>>> +    static int my_critical_function(SomeState *s, void *data)
>>> +    {
>>> +        QEMU_LOCK_GUARD(&s->lock);
>>> +        do_thing1(data);
>>> +        if (check_state2(data)) {
>>> +            return -1;
>>> +        }
>>> +        do_thing3(data);
>>> +        return 0;
>>> +    }
>>
>> For more clearness, maybe add an equivalent code with qemu_mutex_lock() / qemu_mutex_unlock(), I mean:
>>
>> The equivalent code without _GUARD macro makes us to carefully put qemu_mutex_unlock() on all exit points:
>>
>> .. code-block:: c
>>
>>      static int my_critical_function(SomeState *s, void *data)
>>      {
>>          qemu_mutex_lock(&s->lock);
>>          do_thing1(data);
>>          if (check_state2(data)) {
>>              qemu_mutex_unlock(&s->lock);
>>              return -1;
>>          }
>>          do_thing3(data);
>>          qemu_mutex_unlock(&s->lock);
>>          return 0;
>>      }
>>
>>> +
>>> +will ensure s->lock is released however the function is exited. There
>>> +are often ``WITH_`` forms of macros which more easily wrap around a
>>> +block inside a function.
>>> +
>>> +.. code-block:: c
>>> +
>>> +    WITH_RCU_READ_LOCK_GUARD() {
>>> +        QTAILQ_FOREACH_RCU(kid, &bus->children, sibling) {
>>> +            err = do_the_thing(kid->child);
>>> +            if (err < 0) {
>>> +                return err;
>>> +            }
>>> +        }
>>> +    }
>>> +
>>
>> and maybe similar here.
> 
> I added the example although I didn't repeat it for the WITH form
> because readers should hopefully have understood the idea with the first
> example.
> 

Agreed, thanks!
diff mbox series

Patch

diff --git a/docs/devel/style.rst b/docs/devel/style.rst
index 0bd01f3fca..b50a981a86 100644
--- a/docs/devel/style.rst
+++ b/docs/devel/style.rst
@@ -657,6 +657,42 @@  that declaration and the new code.
 
 See :ref:`qom` for more details.
 
+QEMU GUARD macros
+=================
+
+QEMU provides a number of ``_GUARD`` macros intended to make the
+handling of multiple exit paths easier. For example using
+``QEMU_LOCK_GUARD`` to take a lock will ensure the lock is released on
+exit from the function.
+
+.. code-block:: c
+
+    static int my_critical_function(SomeState *s, void *data)
+    {
+        QEMU_LOCK_GUARD(&s->lock);
+        do_thing1(data);
+        if (check_state2(data)) {
+            return -1;
+        }
+        do_thing3(data);
+        return 0;
+    }
+
+will ensure s->lock is released however the function is exited. There
+are often ``WITH_`` forms of macros which more easily wrap around a
+block inside a function.
+
+.. code-block:: c
+
+    WITH_RCU_READ_LOCK_GUARD() {
+        QTAILQ_FOREACH_RCU(kid, &bus->children, sibling) {
+            err = do_the_thing(kid->child);
+            if (err < 0) {
+                return err;
+            }
+        }
+    }
+
 Error handling and reporting
 ============================