<html><head></head><body>Hi,<br><br>In hindsight, those two functions are not used, and probably should not be used, outside the core, so they could be de-inlined and split from vlc_control_cancel() as internal functions.<br><br>The Linux/Android atomic_wait backend only supports 32-bits. I have untested implementations for FreeBSD and OpenBSD with the same limitation, that I could post.<br><br><div class="gmail_quote">Le 10 février 2020 10:36:37 GMT+02:00, Steve Lhomme <robux4@ycbcr.xyz> a écrit :<blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
<pre class="k9mail">On 2020-02-07 16:38, Rémi Denis-Courmont wrote:<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #729fcf; padding-left: 1ex;">Hi,<br><br>Where do you get that from? <br></blockquote><br> From <br><a href="https://code.videolan.org/videolan/vlc/blob/master/src/win32/thread.c#L68">https://code.videolan.org/videolan/vlc/blob/master/src/win32/thread.c#L68</a><br><br>It's not clear for Android though<br><a href="https://code.videolan.org/videolan/vlc/blob/master/src/android/thread.c#L142">https://code.videolan.org/videolan/vlc/blob/master/src/android/thread.c#L142</a><br>But in the end that's also what is used:<br><a href="https://code.videolan.org/videolan/vlc/blob/master/src/android/thread.c#L287">https://code.videolan.org/videolan/vlc/blob/master/src/android/thread.c#L287</a><br><br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #729fcf; padding-left: 1ex;">The Windows backend can handle any suitably <br>aligned 32-bits blob. And unsigned poses less concerns with wrapping.<br></blockquote><br>I think it's better to have this in the signature, even if it involves a <br>conscious cast when the types don't exactly match.<br><br>There's currently no check on the caller (ultimately the caller is <br>vlc_cancel_addr_prepare). Resulting in passing a plain unsigned pointer <br>where an atomic 32 bits pointer (or can it be another size on other <br>platforms ?) is expected.<br><br>The only check there is is that the condition value one can fit in the <br>type that is assumed on all the backends. And even then if it was an <br>atomic char it would probably pass the static assert but fail at runtime.<br><br>I'm OK with using atomic_uint instead of atomic_int for this patch, and <br>update the backends accordingly. We only set the value to 1.<br><br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #729fcf; padding-left: 1ex;">Le 7 février 2020 17:02:08 GMT+02:00, Steve Lhomme <robux4@ycbcr.xyz> a <br>écrit :<br><br> Not a atomic_uint.<hr> include/vlc_threads.h | 4 ++--<br> src/android/thread.c | 4 ++--<br> src/misc/threads.c | 26 +++++++++++++-------------<br> src/win32/thread.c | 4 ++--<br> 4 files changed, 19 insertions(+), 19 deletions(-)<br><br> diff --git a/include/vlc_threads.h b/include/vlc_threads.h<br> index 8a39c610870..7ae25931b72 100644<br> --- a/include/vlc_threads.h<br> +++ b/include/vlc_threads.h<br> @@ -1028,8 +1028,8 @@ enum<br> {<br> VLC_CLEANUP_PUSH,<br> VLC_CLEANUP_POP,<br> - VLC_CANCEL_ADDR_SET,<br> - VLC_CANCEL_ADDR_CLEAR,<br> + VLC_CANCEL_ADDR_SET, /* atomic_int* */<br> + VLC_CANCEL_ADDR_CLEAR, /* atomic_int* */<br> };<br> <br> #if defined (LIBVLC_USE_PTHREAD_CLEANUP)<br> diff --git a/src/android/thread.c b/src/android/thread.c<br> index ca715418d41..aec539b4f03 100644<br> --- a/src/android/thread.c<br> +++ b/src/android/thread.c<br> @@ -341,7 +341,7 @@ void vlc_control_cancel(int cmd, ...)<br> {<br> case VLC_CANCEL_ADDR_SET:<br> {<br> - void *addr = va_arg(ap, void *);<br> + void *addr = va_arg(ap, atomic_int *);<br> <br> vlc_mutex_lock(&th->wait.lock);<br> assert(th->wait.addr == NULL);<br> @@ -352,7 +352,7 @@ void vlc_control_cancel(int cmd, ...)<br> <br> case VLC_CANCEL_ADDR_CLEAR:<br> {<br> - void *addr = va_arg(ap, void *);<br> + void *addr = va_arg(ap, atomic_int *);<br> <br> vlc_mutex_lock(&th->wait.lock);<br> assert(th->wait.addr == addr);<br> diff --git a/src/misc/threads.c b/src/misc/threads.c<br> index 501a17ac9b3..87c3da03d9e 100644<br> --- a/src/misc/threads.c<br> +++ b/src/misc/threads.c<br> @@ -149,7 +149,7 @@ bool vlc_mutex_marked(const vlc_mutex_t *mutex)<br> #if defined(LIBVLC_NEED_SLEEP) || defined(LIBVLC_NEED_CONDVAR)<br> #include <stdatomic.h><br> <br> -static void vlc_cancel_addr_prepare(void *addr)<br> +static void vlc_cancel_addr_prepare(atomic_int *addr)<br> {<br> /* Let thread subsystem on address to broadcast for cancellation */<br> vlc_cancel_addr_set(addr);<br> @@ -159,7 +159,7 @@ static void vlc_cancel_addr_prepare(void *addr)<br> vlc_cleanup_pop();<br> }<br> <br> -static void vlc_cancel_addr_finish(void *addr)<br> +static void vlc_cancel_addr_finish(atomic_int *addr)<br> {<br> vlc_cancel_addr_clear(addr);<br> /* Act on cancellation as potential wake-up source */<br> @@ -193,14 +193,14 @@ void (vlc_tick_sleep)(vlc_tick_t delay)<br> #ifdef LIBVLC_NEED_CONDVAR<br> #include <stdalign.h><br> <br> -static inline atomic_uint *vlc_cond_value(vlc_cond_t *cond)<br> +static inline atomic_int *vlc_cond_value(vlc_cond_t *cond)<br> {<br> /* XXX: ugly but avoids including stdatomic.h in vlc_threads.h */<br> - static_assert (sizeof (cond->value) <= sizeof (atomic_uint),<br> + static_assert (sizeof (cond->value) <= sizeof (atomic_int),<br> "Size mismatch!");<br> - static_assert ((alignof (cond->value) % alignof (atomic_uint)) == 0,<br> + static_assert ((alignof (cond->value) % alignof (atomic_int)) == 0,<br> "Alignment mismatch");<br> - return (atomic_uint *)&cond->value;<br> + return (atomic_int *)&cond->value;<br> }<br> <br> void vlc_cond_init(vlc_cond_t *cond)<br> @@ -247,7 +247,7 @@ void vlc_cond_broadcast(vlc_cond_t *cond)<br> <br> void vlc_cond_wait(vlc_cond_t *cond, vlc_mutex_t *mutex)<br> {<br> - unsigned value = atomic_load_explicit(vlc_cond_value(cond),<br> + int value = atomic_load_explicit(vlc_cond_value(cond),<br> memory_order_relaxed);<br> while (value & 1)<br> {<br> @@ -258,20 +258,20 @@ void vlc_cond_wait(vlc_cond_t *cond, vlc_mutex_t *mutex)<br> value++;<br> }<br> <br> - vlc_cancel_addr_prepare(&cond->value);<br> + vlc_cancel_addr_prepare(vlc_cond_value(cond));<br> vlc_mutex_unlock(mutex);<br> <br> vlc_atomic_wait(&cond->value, value);<br> <br> vlc_mutex_lock(mutex);<br> - vlc_cancel_addr_finish(&cond->value);<br> + vlc_cancel_addr_finish(vlc_cond_value(cond));<br> }<br> <br> static int vlc_cond_wait_delay(vlc_cond_t *cond, vlc_mutex_t *mutex,<br> vlc_tick_t delay)<br> {<br> - unsigned value = atomic_load_explicit(vlc_cond_value(cond),<br> - memory_order_relaxed);<br> + int value = atomic_load_explicit(vlc_cond_value(cond),<br> + memory_order_relaxed);<br> while (value & 1)<br> {<br> if (atomic_compare_exchange_weak_explicit(vlc_cond_value(cond), &value,<br> @@ -281,7 +281,7 @@ static int vlc_cond_wait_delay(vlc_cond_t *cond, vlc_mutex_t *mutex,<br> value++;<br> }<br> <br> - vlc_cancel_addr_prepare(&cond->value);<br> + vlc_cancel_addr_prepare(vlc_cond_value(cond));<br> vlc_mutex_unlock(mutex);<br> <br> if (delay > 0)<br> @@ -290,7 +290,7 @@ static int vlc_cond_wait_delay(vlc_cond_t *cond, vlc_mutex_t *mutex,<br> value = 0;<br> <br> vlc_mutex_lock(mutex);<br> - vlc_cancel_addr_finish(&cond->value);<br> + vlc_cancel_addr_finish(vlc_cond_value(cond));<br> <br> return value ? 0 : ETIMEDOUT;<br> }<br> diff --git a/src/win32/thread.c b/src/win32/thread.c<br> index 6af2eb83852..10c36e9ed82 100644<br> --- a/src/win32/thread.c<br> +++ b/src/win32/thread.c<br> @@ -669,7 +669,7 @@ void vlc_control_cancel (int cmd, ...)<br> <br> case VLC_CANCEL_ADDR_SET:<br> {<br> - void *addr = va_arg(ap, void *);<br> + void *addr = va_arg(ap, atomic_int *);<br> <br> EnterCriticalSection(&th->wait.lock);<br> assert(th->wait.addr == NULL);<br> @@ -680,7 +680,7 @@ void vlc_control_cancel (int cmd, ...)<br> <br> case VLC_CANCEL_ADDR_CLEAR:<br> {<br> - void *addr = va_arg(ap, void *);<br> + void *addr = va_arg(ap, atomic_int *);<br> <br> EnterCriticalSection(&th->wait.lock);<br> assert(th->wait.addr == addr);<br><br><br>-- <br>Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser <br>ma brièveté.<hr>vlc-devel mailing list<br>To unsubscribe or modify your subscription options:<br><a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br><br></blockquote><hr>vlc-devel mailing list<br>To unsubscribe or modify your subscription options:<br><a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a></pre></blockquote></div><br>-- <br>Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.</body></html>