2009/11/4 Johannes Schindelin <Johannes.Schindelin@xxxxxx>: > 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html