[waffle] [PATCH 2/3] wcore: rework non-compatible mutex initialization

Emil Velikov emil.l.velikov at gmail.com
Fri Mar 20 05:52:10 PDT 2015


On 20 March 2015 at 11:46, Jose Fonseca <jfonseca at vmware.com> wrote:
> On 19/03/15 22:26, Emil Velikov wrote:
>>
>> C11 does not specify a static initializer, based on the idea that the
>> a mutex will be platform and/or implementation dependent. As such the
>> alternative solution is to initialize the mutex with mtx_init().
>> This will allow us to remove the transition hack, and in due time move
>> to the system's C11 threads implementation.
>>
>> Signed-off-by: Emil Velikov <emil.l.velikov at gmail.com>
>> ---
>>   src/waffle/core/wcore_display.c | 6 +++---
>>   src/waffle/core/wcore_display.h | 3 ++-
>>   2 files changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/waffle/core/wcore_display.c
>> b/src/waffle/core/wcore_display.c
>> index 2feeeba..49aeae6 100644
>> --- a/src/waffle/core/wcore_display.c
>> +++ b/src/waffle/core/wcore_display.c
>> @@ -34,14 +34,14 @@ wcore_display_init(struct wcore_display *self,
>>                      struct wcore_platform *platform)
>>   {
>>       static size_t id_counter = 0;
>> -    static mtx_t mutex = _MTX_INITIALIZER_NP;
>
>
> IIUC, this mutex protecting the static id_counter above, not the
> wcore_display object, or anything, so moving mtx to display seems incorrect.
>
> The ideal solution here would be to use C11 atomics, but that's a bunch of
> extra portibility code to maintain...  The portable way of fixing this with
> C11 threads is using once_flag.
>
Agreed, using C11 atomics was something that came to mind. Although
I'd rather avoid adding another more compat layer.

I see that keeping the mutex within struct wcore_display was a bad
idea. And it's the one lead me to drop the call_once in the first
place. Upon closer look it seems that
wcore_error_unittest.c:test_wcore_error_thread_local() will need a
similar treatment.

As always you're a star Jose. Thanks.

Emil


More information about the waffle mailing list