[vlc-devel] [PATCH 4/4] threads: remove uneeded vlc_cond_value()

[Rémi Denis-Courmont]

Hi,

They're not specific to cond_init. The assertions should be in global context IMO (nit). Otherwise LGTM

Le 10 février 2020 11:02:22 GMT+02:00, Steve Lhomme <robux4 at ycbcr.xyz> a écrit :
>We can use the atomic variable directly without any cast.
>
>Only the C++ user need to be checked for the proper storage
>size/alignment of the variable they pass.
>---
> src/misc/threads.c | 35 ++++++++++++++---------------------
> 1 file changed, 14 insertions(+), 21 deletions(-)
>
>diff --git a/src/misc/threads.c b/src/misc/threads.c
>index 304ae972199..92d226cfb72 100644
>--- a/src/misc/threads.c
>+++ b/src/misc/threads.c
>@@ -208,20 +208,15 @@ void (vlc_tick_sleep)(vlc_tick_t delay)
> #ifdef LIBVLC_NEED_CONDVAR
> #include <stdalign.h>
> 
>-static inline atomic_uint *vlc_cond_value(vlc_cond_t *cond)
>+void vlc_cond_init(vlc_cond_t *cond)
> {
>/* Don't use C++ atomic types in vlc_threads.h for the C atomic storage
>*/
>+    /* Initial value is irrelevant but set it for happy debuggers */
>     static_assert (sizeof (cond->cpp_value) <= sizeof (cond->value),
>                    "Size mismatch!");
>static_assert ((alignof (cond->cpp_value) % alignof (cond->value)) ==
>0,
>                    "Alignment mismatch");
>-    return &cond->value;
>-}
>-
>-void vlc_cond_init(vlc_cond_t *cond)
>-{
>-    /* Initial value is irrelevant but set it for happy debuggers */
>-    atomic_init(vlc_cond_value(cond), 0);
>+    atomic_init(&cond->value, 0);
> }
> 
> void vlc_cond_init_daytime(vlc_cond_t *cond)
>@@ -232,7 +227,7 @@ void vlc_cond_init_daytime(vlc_cond_t *cond)
> void vlc_cond_destroy(vlc_cond_t *cond)
> {
>     /* Tempting sanity check but actually incorrect:
>-    assert((atomic_load_explicit(vlc_cond_value(cond),
>+    assert((atomic_load_explicit(&cond->value,
>                                  memory_order_relaxed) & 1) == 0);
> * Due to timeouts and spurious wake-ups, the futex value can look like
>      * there are waiters, even though there are none. */
>@@ -250,53 +245,51 @@ void vlc_cond_signal(vlc_cond_t *cond)
>  * - cnd_signal() sets the futex to the equal-or-next odd value, while
>      * - cnd_wait() sets the futex to the equal-or-next even value.
>      **/
>-    atomic_fetch_or_explicit(vlc_cond_value(cond), 1,
>memory_order_relaxed);
>+    atomic_fetch_or_explicit(&cond->value, 1, memory_order_relaxed);
>     vlc_atomic_notify_one(&cond->value);
> }
> 
> void vlc_cond_broadcast(vlc_cond_t *cond)
> {
>-    atomic_fetch_or_explicit(vlc_cond_value(cond), 1,
>memory_order_relaxed);
>+    atomic_fetch_or_explicit(&cond->value, 1, memory_order_relaxed);
>     vlc_atomic_notify_all(&cond->value);
> }
> 
> void vlc_cond_wait(vlc_cond_t *cond, vlc_mutex_t *mutex)
> {
>-    unsigned value = atomic_load_explicit(vlc_cond_value(cond),
>-                                     memory_order_relaxed);
>+    unsigned value = atomic_load_explicit(&cond->value,
>memory_order_relaxed);
>     while (value & 1)
>     {
>-        if
>(atomic_compare_exchange_weak_explicit(vlc_cond_value(cond), &value,
>+        if (atomic_compare_exchange_weak_explicit(&cond->value,
>&value,
>                                                   value + 1,
>                                                  memory_order_relaxed,
>                                                 memory_order_relaxed))
>             value++;
>     }
> 
>-    vlc_cancel_addr_prepare(vlc_cond_value(cond));
>+    vlc_cancel_addr_prepare(&cond->value);
>     vlc_mutex_unlock(mutex);
> 
>     vlc_atomic_wait(&cond->value, value);
> 
>     vlc_mutex_lock(mutex);
>-    vlc_cancel_addr_finish(vlc_cond_value(cond));
>+    vlc_cancel_addr_finish(&cond->value);
> }
> 
> static int vlc_cond_wait_delay(vlc_cond_t *cond, vlc_mutex_t *mutex,
>                                vlc_tick_t delay)
> {
>-    unsigned value = atomic_load_explicit(vlc_cond_value(cond),
>-                                          memory_order_relaxed);
>+    unsigned value = atomic_load_explicit(&cond->value,
>memory_order_relaxed);
>     while (value & 1)
>     {
>-        if
>(atomic_compare_exchange_weak_explicit(vlc_cond_value(cond), &value,
>+        if (atomic_compare_exchange_weak_explicit(&cond->value,
>&value,
>                                                   value + 1,
>                                                  memory_order_relaxed,
>                                                 memory_order_relaxed))
>             value++;
>     }
> 
>-    vlc_cancel_addr_prepare(vlc_cond_value(cond));
>+    vlc_cancel_addr_prepare(&cond->value);
>     vlc_mutex_unlock(mutex);
> 
>     if (delay > 0)
>@@ -305,7 +298,7 @@ static int vlc_cond_wait_delay(vlc_cond_t *cond,
>vlc_mutex_t *mutex,
>         value = 0;
> 
>     vlc_mutex_lock(mutex);
>-    vlc_cancel_addr_finish(vlc_cond_value(cond));
>+    vlc_cancel_addr_finish(&cond->value);
> 
>     return value ? 0 : ETIMEDOUT;
> }
>-- 
>2.17.1
>
>_______________________________________________
>vlc-devel mailing list
>To unsubscribe or modify your subscription options:
>https://mailman.videolan.org/listinfo/vlc-devel

-- 
Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20200210/f3c7f2ed/attachment-0001.html>