[Mesa-dev] [PATCH 03/25] threads: update for late C11 changes

[Jose Fonseca]

On 26/10/17 13:55, Jose Fonseca wrote:
> On 23/10/17 20:15, Emil Velikov wrote:
>> Hi Nicolai,
>>
>> On 22 October 2017 at 20:07, Nicolai Hähnle <nhaehnle at gmail.com> wrote:
>>> From: Nicolai Hähnle <nicolai.haehnle at amd.com>
>>>
>>> C11 threads were changed to use struct timespec instead of xtime, and
>>> thrd_sleep got a second argument.
>>>
>> As xtime was replaced with timespec there's a couple of odd bits in 
>> the code.
>>
>>
>>> See http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1554.htm and
>>> http://en.cppreference.com/w/c/thread/{thrd_sleep,cnd_timedwait,mtx_timedlock} 
>>>
>>>
>>> Note that cnd_timedwait is spec'd to be relative to TIME_UTC / 
>>> CLOCK_REALTIME.
>>>
>>> Cc: Jose Fonseca <jfonseca at vmware.com>
>>> ---
>>>   include/c11/threads.h           | 11 -----------
>>>   include/c11/threads_posix.h     | 39 
>>> +++++++++++++++------------------------
>>>   include/c11/threads_win32.h     | 37 
>>> +++++++++++++++++++------------------
>>>   src/egl/drivers/dri2/egl_dri2.c | 24 +++++++++++++-----------
>>>   4 files changed, 47 insertions(+), 64 deletions(-)
>>>
>>> diff --git a/include/c11/threads.h b/include/c11/threads.h
>>> index 573348d8091..3c3f23a8ab8 100644
>>> --- a/include/c11/threads.h
>>> +++ b/include/c11/threads.h
>>> @@ -23,42 +23,31 @@
>>>    * FITNESS FOR A PARTICULAR PURPOSE, TITLE AND NON-INFRINGEMENT. IN 
>>> NO EVENT
>>>    * SHALL THE COPYRIGHT HOLDERS OR ANYONE DISTRIBUTING THE SOFTWARE 
>>> BE LIABLE
>>>    * FOR ANY DAMAGES OR OTHER LIABILITY, WHETHER IN CONTRACT, TORT OR 
>>> OTHERWISE,
>>>    * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE 
>>> USE OR OTHER
>>>    * DEALINGS IN THE SOFTWARE.
>>>    */
>>>   #ifndef EMULATED_THREADS_H_INCLUDED_
>>>   #define EMULATED_THREADS_H_INCLUDED_
>>>
>>>   #include <time.h>
>>> -#ifdef _MSC_VER
>>> -#include <thr/xtimec.h>  // for xtime
>>> -#endif
>>>
>>>   #ifndef TIME_UTC
>>>   #define TIME_UTC 1
>>>   #endif
>>>
>>>   #include "c99_compat.h" /* for `inline` */
>>>
>>>   /*---------------------------- types ----------------------------*/
>>>   typedef void (*tss_dtor_t)(void*);
>>>   typedef int (*thrd_start_t)(void*);
>>>
>>> -#ifndef _MSC_VER
>>> -struct xtime {
>>> -    time_t sec;
>>> -    long nsec;
>>> -};
>>> -typedef struct xtime xtime;
>>> -#endif
>>> -
>>
>> We don't have a fall-back declaration of the struct, yet we use it
>> below and provide a timespec_get() implementation.
>> I'd imagine you haven't tested this on Windows (hence Jose in CC)?
>>
>> Quick search suggests that MSVC 2015 was the first one that introduces
>> the struct and timespec_get.
>>
>> If we're safe as-is, please add a comment with some details - I'd
>> imagine Jose had better knowledge in the area.
>>
>>
>>>   /*-------------------- 7.25.7 Time functions --------------------*/
>>>   // 7.25.6.1
>>> +#if 0
>> I'd just drop the hunk mentioning that timespec_get() is part of time.h
>>
>>
>> Thank
>> Emil
> 
> If there's doubt, I suggest testing Visual Studio with AppVeyor by 
> pushing the changes as a feature branch to FDO's git -- I believe that 
> should trigger an AppVeyor build.  (Push to a github repos hooked into 
> Appveyor, depending on what people are more confortable with.)
> 
> Alternatively reach out to Brian or Roland.
> 
> I'm currently on PTO, so I'm afraid I don't have the time nor a devel 
> setup to try this out..
> 
> Jose
I forgot to say: assuming we don't need xtime anymore, at a glance, the 
patch looks ok to me.


Jose