[vlc-devel] [PATCH 1/6] os2: implement vlc_once()

Kamil Rytarowski n54 at gmx.com
Tue Apr 3 13:07:50 CEST 2018


On 03.04.2018 12:48, Rémi Denis-Courmont wrote:
> Le 3 avril 2018 12:35:15 GMT+02:00, Kamil Rytarowski <n54 at gmx.com> a écrit :
>> On 03.04.2018 08:39, Rémi Denis-Courmont wrote:
>>> Le mardi 3 avril 2018, 00:12:53 EEST Kamil Rytarowski a écrit :
>>>> On 23.03.2018 09:54, Rémi Denis-Courmont wrote:
>>>>> Le jeudi 22 mars 2018, 16:41:29 EET KO Myung-Hun a écrit :
>>>>>> ---
>>>>>>
>>>>>>  include/vlc_threads.h |  6 ++++++
>>>>>>  src/os2/thread.c      | 17 +++++++++++++++++
>>>>>>  2 files changed, 23 insertions(+)
>>>>>>
>>>>>> diff --git a/include/vlc_threads.h b/include/vlc_threads.h
>>>>>> index 37054a3ded..b7505da266 100644
>>>>>> --- a/include/vlc_threads.h
>>>>>> +++ b/include/vlc_threads.h
>>>>>> @@ -125,6 +125,12 @@ typedef struct
>>>>>>
>>>>>>  #define VLC_STATIC_COND { NULLHANDLE, 0, NULLHANDLE, 0 }
>>>>>>  #define LIBVLC_NEED_SEMAPHORE
>>>>>>  #define LIBVLC_NEED_RWLOCK
>>>>>>
>>>>>> +typedef struct
>>>>>> +{
>>>>>> +    unsigned done;
>>>>>> +    vlc_mutex_t mutex;
>>>>>> +} vlc_once_t;
>>>>>> +#define VLC_STATIC_ONCE { 0, VLC_STATIC_MUTEX }
>>>>>>
>>>>>>  typedef struct vlc_threadvar *vlc_threadvar_t;
>>>>>>  typedef struct vlc_timer *vlc_timer_t;
>>>>>>
>>>>>> diff --git a/src/os2/thread.c b/src/os2/thread.c
>>>>>> index 8f7b335ea4..52f58023b8 100644
>>>>>> --- a/src/os2/thread.c
>>>>>> +++ b/src/os2/thread.c
>>>>>> @@ -427,6 +427,23 @@ int vlc_cond_timedwait_daytime (vlc_cond_t
>>>>>> *p_condvar,
>>>>>> vlc_mutex_t *p_mutex, return vlc_cond_wait_common (p_condvar,
>> p_mutex,
>>>>>> ulTimeout);
>>>>>>
>>>>>>  }
>>>>>>
>>>>>> +void vlc_once(vlc_once_t *once, void (*cb)(void))
>>>>>> +{
>>>>>> +    if( !once->done )
>>>>>
>>>>> This looks like a violation of the memory model.
>>>>
>>>> This patch is correct.
>>>
>>> No, it is not.
>>>
>>
>> Show a testcase breaking it with correct usage of this API, otherwise
>> this claim is to be treated as unreasonable and shall be ignored, and
>> patch merged.
> 
> Such a process could work in a fully defined language, such as single threaded Java. C is not a fully defined language in general and multithreaded C in particular.
> 
> For such a small function, it is pretty damn obvious that access to pto_done is not according to either types of accesses defined by the POSIX memory model. It only works if there is no race, but the whole point of the function is to handle potential races.
> 

A testcase is still missing.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 850 bytes
Desc: OpenPGP digital signature
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20180403/a226fd67/attachment.sig>


More information about the vlc-devel mailing list