[vlc-devel] [PATCH v2] thread: fix compilation with clang 7 or older

Steve Lhomme robux4 at ycbcr.xyz
Mon Mar 2 10:17:06 CET 2020


On 2020-03-02 10:05, Marvin Scholz wrote:
> 
> 
> On 2 Mar 2020, at 9:28, Steve Lhomme wrote:
> 
>> On 2020-03-02 8:53, Marvin Scholz wrote:
>>> C11's atomic_load_explicit did not allow a const qualified first
>>> argument, even though it never modifies it (see DR459).
>>> Clang versions 7 or older did implement this quite strictly and
>>> therefore errors when compiling this code:
>>>
>>>    address argument to atomic operation must be a pointer to non-const
>>>    _Atomic type ('const _Atomic(const void *) *' invalid)
>>>
>>> See https://reviews.llvm.org/D47618 and https://reviews.llvm.org/D47613
>>> ---
>>>   src/misc/threads.c | 12 +++++++++++-
>>>   1 file changed, 11 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/src/misc/threads.c b/src/misc/threads.c
>>> index ea78e37453..db90a27240 100644
>>> --- a/src/misc/threads.c
>>> +++ b/src/misc/threads.c
>>> @@ -135,11 +135,21 @@ static _Thread_local char thread_self[1];
>>>    bool vlc_mutex_held(const vlc_mutex_t *mtx)
>>>   {
>>> +#if defined(__clang__) && !defined(__apple_build_version__) && 
>>> (__clang_major__ < 8)
>>
>> The Apple 7- versions don't have this issue ?
>>
>> IMO it's OK not to add the distinction for this compiler flavor. 
>> Otherwsise we'd have to check which version from which vendor has the 
>> issue.
>>
>> LGTM otherwise.
> 
> Apple Clang uses a different versioning compared to the Open Source 
> releases.
> I can’t easily check if whatever versions might correspond to the 
> affected ones
> have this issue.

OK, you should probably add a comment about that then.

> I tried it with Xcode 10 and it was fine. Given that Xcode 10 is the 
> oldest Xcode
> version we currently support for 4.x, I believe there is no need to check.
> 
>>
>>> +    /* The explicit cast to non-const is needed to workaround a clang
>>> +     * error with clang 7 or lower, as atomic_load_explicit in C11 
>>> did not
>>> +     * allow its first argument to be const-qualified (see DR459)
>>> +     */
>>> +    vlc_mutex_t *tmp_mtx = (vlc_mutex_t *)mtx;
>>> +#else
>>> +    const vlc_mutex_t *tmp_mtx = mtx;
>>> +#endif
>>> +
>>>       /* This comparison is thread-safe:
>>>        * Even though other threads may modify the owner field at any 
>>> time,
>>>        * they will never make it compare equal to the calling thread.
>>>        */
>>> -    return THREAD_SELF == atomic_load_explicit(&mtx->owner,
>>> +    return THREAD_SELF == atomic_load_explicit(&tmp_mtx->owner,
>>>                                                  memory_order_relaxed);
>>>   }
>>>  -- 2.21.1 (Apple Git-122.3)
>>>
>>> _______________________________________________
>>> 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
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel


More information about the vlc-devel mailing list