diff mbox series

[FFmpeg-devel] compat/w32pthreads: propagate the return value of SleepConditionVariableSRW()

Message ID 20200117014120.22427-1-jamrial@gmail.com
State New
Headers show
Series [FFmpeg-devel] compat/w32pthreads: propagate the return value of SleepConditionVariableSRW()
Related show

Checks

Context Check Description
andriy/ffmpeg-patchwork pending
andriy/ffmpeg-patchwork success Applied patch
andriy/ffmpeg-patchwork success Configure finished
andriy/ffmpeg-patchwork success Make finished
andriy/ffmpeg-patchwork success Make fate finished

Commit Message

James Almer Jan. 17, 2020, 1:41 a.m. UTC
Signed-off-by: James Almer <jamrial@gmail.com>
---
 compat/w32pthreads.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Hendrik Leppkes Jan. 17, 2020, 7:41 a.m. UTC | #1
On Fri, Jan 17, 2020 at 2:41 AM James Almer <jamrial@gmail.com> wrote:
>
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  compat/w32pthreads.h | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/compat/w32pthreads.h b/compat/w32pthreads.h
> index 21acfd2ba1..1ac4267c92 100644
> --- a/compat/w32pthreads.h
> +++ b/compat/w32pthreads.h
> @@ -152,8 +152,7 @@ static inline int pthread_cond_broadcast(pthread_cond_t *cond)
>
>  static inline int pthread_cond_wait(pthread_cond_t *cond, pthread_mutex_t *mutex)
>  {
> -    SleepConditionVariableSRW(cond, mutex, INFINITE, 0);
> -    return 0;
> +    return !SleepConditionVariableSRW(cond, mutex, INFINITE, 0);
>  }
>

The return of SleepConditionVariableSRW is not very meaningful, it
only returns success or failure, if we wanted to mimmick proper
pthread API, we could map a few common returns from GetLastError() to
their appropriate values.

- Hendrik
James Almer Jan. 17, 2020, 10:24 p.m. UTC | #2
On 1/17/2020 4:41 AM, Hendrik Leppkes wrote:
> On Fri, Jan 17, 2020 at 2:41 AM James Almer <jamrial@gmail.com> wrote:
>>
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>>  compat/w32pthreads.h | 3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/compat/w32pthreads.h b/compat/w32pthreads.h
>> index 21acfd2ba1..1ac4267c92 100644
>> --- a/compat/w32pthreads.h
>> +++ b/compat/w32pthreads.h
>> @@ -152,8 +152,7 @@ static inline int pthread_cond_broadcast(pthread_cond_t *cond)
>>
>>  static inline int pthread_cond_wait(pthread_cond_t *cond, pthread_mutex_t *mutex)
>>  {
>> -    SleepConditionVariableSRW(cond, mutex, INFINITE, 0);
>> -    return 0;
>> +    return !SleepConditionVariableSRW(cond, mutex, INFINITE, 0);
>>  }
>>
> 
> The return of SleepConditionVariableSRW is not very meaningful, it
> only returns success or failure, if we wanted to mimmick proper
> pthread API, we could map a few common returns from GetLastError() to
> their appropriate values.

I noticed when looking at the patch Marton sent that upd.c has the only
case of pthread_cond_wait() where we check the return value, but since
it only cares if it failed or not, we can ignore which error it produces
for now, IMO.

Without this change, once this code starts working with w32threads, any
potential error will be ignored.

> 
> - Hendrik
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
diff mbox series

Patch

diff --git a/compat/w32pthreads.h b/compat/w32pthreads.h
index 21acfd2ba1..1ac4267c92 100644
--- a/compat/w32pthreads.h
+++ b/compat/w32pthreads.h
@@ -152,8 +152,7 @@  static inline int pthread_cond_broadcast(pthread_cond_t *cond)
 
 static inline int pthread_cond_wait(pthread_cond_t *cond, pthread_mutex_t *mutex)
 {
-    SleepConditionVariableSRW(cond, mutex, INFINITE, 0);
-    return 0;
+    return !SleepConditionVariableSRW(cond, mutex, INFINITE, 0);
 }
 
 static inline int pthread_cond_signal(pthread_cond_t *cond)