[vlc-devel] [PATCH 3/4] threads: store the condition variable value as an atomic_uint with LIBVLC_NEED_CONDVAR

[Rémi Denis-Courmont]

Hi,

I was referring to (not) needlessly increasing the size of vlc_cond_t, which would occur if union alignment is 8 or 16 bytes.

Regardless, the current CV implementation has a wake-up loss bug, and I need to change the definition of vlc_cond_t to address it, so this is beating a dead horse.

Le 10 février 2020 14:06:06 GMT+02:00, Steve Lhomme <robux4 at ycbcr.xyz> a écrit :
>On 2020-02-10 11:52, Rémi Denis-Courmont wrote:
>> Ok. Do we have any relevant platform where unions have high alignment
>
>> than uint regardless of their members?
>
>No idea. But it's probably safer not too assume too many things.
>
>Also we're assuming the alignment of the structure in C code is the
>same 
>as the structure in C++. Maybe it's not.
>
>> Le 10 février 2020 11:02:21 GMT+02:00, Steve Lhomme
><robux4 at ycbcr.xyz> a 
>> écrit :
>> 
>>     We can include stdatomic.h in the headers, we do it in many
>places. But we
>>     can't use it with C++ because the std:atomic_int storage is
>likely different
>>     and can be initialized/released the same way.
>> 
>>     We change the static assert in the core to make sure the type in
>the union that
>>     will be used for storage by C++ modules is correct.
>>    
>------------------------------------------------------------------------
>>       include/vlc_threads.h | 12 +++++++++++-
>>       src/misc/threads.c    |  8 ++++----
>>       2 files changed, 15 insertions(+), 5 deletions(-)
>> 
>>     diff --git a/include/vlc_threads.h b/include/vlc_threads.h
>>     index f30f0699e37..1a05e57b82c 100644
>>     --- a/include/vlc_threads.h
>>     +++ b/include/vlc_threads.h
>>     @@ -351,9 +351,19 @@ typedef struct vlc_timer *vlc_timer_t;
>>       #endif
>>       
>>       #ifdef LIBVLC_NEED_CONDVAR
>>     +#ifndef __cplusplus
>>     +#include <stdatomic.h>
>>     +#endif
>>     +
>>     +
>>       typedef struct
>>       {
>>     -    unsigned value;
>>     +    union {
>>     +#ifndef __cplusplus
>>     +        atomic_uint value;
>>     +#endif
>>     +        int        cpp_value;
>>     +    };
>>       } vlc_cond_t;
>>       # define VLC_STATIC_COND { 0 }
>>       #endif
>>     diff --git a/src/misc/threads.c b/src/misc/threads.c
>>     index 18855e85bd4..304ae972199 100644
>>     --- a/src/misc/threads.c
>>     +++ b/src/misc/threads.c
>>     @@ -210,12 +210,12 @@ void (vlc_tick_sleep)(vlc_tick_t delay)
>>       
>>       static inline atomic_uint *vlc_cond_value(vlc_cond_t *cond)
>>       {
>>     -    /* XXX: ugly but avoids including stdatomic.h in
>vlc_threads.h */
>>     -    static_assert (sizeof (cond->value) <= sizeof (atomic_uint),
>>     +    /* Don't use C++ atomic types in vlc_threads.h for the C
>atomic storage */
>>     +    static_assert (sizeof (cond->cpp_value) <= sizeof
>(cond->value),
>>                          "Size mismatch!");
>>     -    static_assert ((alignof (cond->value) % alignof
>(atomic_uint)) == 0,
>>     +    static_assert ((alignof (cond->cpp_value) % alignof
>(cond->value)) == 0,
>>                          "Alignment mismatch");
>>     -    return (atomic_uint *)&cond->value;
>>     +    return &cond->value;
>>       }
>>       
>>       void vlc_cond_init(vlc_cond_t *cond)
>> 
>> 
>> -- 
>> Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez
>excuser 
>> ma brièveté.
>> 
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel
>> 
>_______________________________________________
>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/41d54234/attachment.html>