From: Andrzej K. Haczewski Date: Wed, 04 Nov 2009 08:17:55 GMT Subject: Re: [PATCH 1/1] MSVC: port pthread code to native Windows threads Message-ID: <16cee31f0911040017x70c7dfeev6696c3ea8f7ffe56@mail.gmail.com> In-Reply-To: 2009/11/4 Johannes Schindelin : > Could you please add the reasoning from the cover letter to this commit > message?  And add a sign-off? Sure, will do so for next submission of that patch. > It is unlikely that an #ifdef "contamination" of this extent will go > through easily, but I have a suggestion that may make your patch both > easier to read and more likely to be accepted into git.git: Try to wrap > the win32 calls into pthread-compatible function signatures.  Then you can > add a compat/win32/pthread.h and not even touch core files of git.git at > all. First of all I didn't want to use wrappers because (if not inlined) they introduce one additional call, that can be avoided with #defines (as you can see even pthread_init can be done with macro). Second reason is that I didn't want to create wrapping structures that would need to be initialized / allocated / tracked. That patch translates pthread calls to purely Win32 calls without anything in between. Here are my reasoning for some of these #ifdefs and what can be done and what can't (without using wrappers): 1. Thread routine has very different signature: void *__cdecl func(void *); /* pthreads */ uint32_t __stdcall func(void *); /* Windows API */ First I thought it might be a problem to do (especially return value, which is different size for 64-bit architectures), but since Git doesn't use return value, it can be done. 2. Initialization of CRITICAL_SECTION and SEMAPHORE (used by condition variables implementation). These need explicit initialization on Windows and can't be done statically with PTHREAD_MUTEX_INITIALIZER and PTHREAD_COND_INITIALIZER. There's no easy way around that (read: it needs wrappers). > Oh, and you definitely do not want to copy-paste err_win_to_posix().  You > definitely want to reuse the existing instance. Yeah, that was lazy, mea culpa. I'll resubmit the patch with some fixes shortly, Andrew