threaded-grep cause msys build failure

threaded-grep cause msys build failure

[Zoltán Füzesi]

Hi,

Building git in msys environment fails:
...
    LINK git.exe
builtin-grep.o: In function `wait_all':
D:\devel\msysgit\git/builtin-grep.c:260: undefined reference to
`pthread_cond_broadcast'
collect2: ld returned 1 exit status
make: *** [git.exe] Error 1

I guess compat/win32/pthread.c misses pthread_cond_broadcast implementation.

Zé

Re: threaded-grep cause msys build failure

[Johannes Sixt]

Zoltán Füzesi schrieb:
> Building git in msys environment fails:
> ...
>     LINK git.exe
> builtin-grep.o: In function `wait_all':
> D:\devel\msysgit\git/builtin-grep.c:260: undefined reference to
> `pthread_cond_broadcast'

Use this. I'll try to find a better solution over the weekend.

diff --git a/Makefile b/Makefile
index c591d70..bbee373 100644
--- a/Makefile
+++ b/Makefile
@@ -183,6 +183,9 @@ all::
 # Define THREADED_DELTA_SEARCH if you have pthreads and wish to exploit
 # parallel delta searching when packing objects.
 #
+# Define THREADED_GREP if you have pthreads and wish to exploit a
+# parallelized grep.
+#
 # Define INTERNAL_QSORT to use Git's implementation of qsort(), which
 # is a simplified version of the merge sort used in glibc. This is
 # recommended if Git triggers O(n^2) behavior in your platform's qsort().
@@ -723,11 +726,13 @@ ifeq ($(uname_S),Linux)
 	NO_STRLCPY = YesPlease
 	NO_MKSTEMPS = YesPlease
 	THREADED_DELTA_SEARCH = YesPlease
+	THREADED_GREP = YesPlease
 endif
 ifeq ($(uname_S),GNU/kFreeBSD)
 	NO_STRLCPY = YesPlease
 	NO_MKSTEMPS = YesPlease
 	THREADED_DELTA_SEARCH = YesPlease
+	THREADED_GREP = YesPlease
 endif
 ifeq ($(uname_S),UnixWare)
 	CC = cc
@@ -782,6 +787,7 @@ ifeq ($(uname_S),Darwin)
 	endif
 	NO_MEMMEM = YesPlease
 	THREADED_DELTA_SEARCH = YesPlease
+	THREADED_GREP = YesPlease
 	USE_ST_TIMESPEC = YesPlease
 endif
 ifeq ($(uname_S),SunOS)
@@ -795,6 +801,7 @@ ifeq ($(uname_S),SunOS)
 	NO_MKSTEMPS = YesPlease
 	NO_REGEX = YesPlease
 	THREADED_DELTA_SEARCH = YesPlease
+	THREADED_GREP = YesPlease
 	ifeq ($(uname_R),5.7)
 		NEEDS_RESOLV = YesPlease
 		NO_IPV6 = YesPlease
@@ -851,6 +858,7 @@ ifeq ($(uname_S),FreeBSD)
 	DIR_HAS_BSD_GROUP_SEMANTICS = YesPlease
 	USE_ST_TIMESPEC = YesPlease
 	THREADED_DELTA_SEARCH = YesPlease
+	THREADED_GREP = YesPlease
 	ifeq ($(shell expr "$(uname_R)" : '4\.'),2)
 		PTHREAD_LIBS = -pthread
 		NO_UINTMAX_T = YesPlease
@@ -865,6 +873,7 @@ ifeq ($(uname_S),OpenBSD)
 	BASIC_CFLAGS += -I/usr/local/include
 	BASIC_LDFLAGS += -L/usr/local/lib
 	THREADED_DELTA_SEARCH = YesPlease
+	THREADED_GREP = YesPlease
 endif
 ifeq ($(uname_S),NetBSD)
 	ifeq ($(shell expr "$(uname_R)" : '[01]\.'),2)
@@ -873,6 +882,7 @@ ifeq ($(uname_S),NetBSD)
 	BASIC_CFLAGS += -I/usr/pkg/include
 	BASIC_LDFLAGS += -L/usr/pkg/lib $(CC_LD_DYNPATH)/usr/pkg/lib
 	THREADED_DELTA_SEARCH = YesPlease
+	THREADED_GREP = YesPlease
 	USE_ST_TIMESPEC = YesPlease
 	NO_MKSTEMPS = YesPlease
 endif
@@ -889,6 +899,7 @@ ifeq ($(uname_S),AIX)
 	BASIC_CFLAGS += -D_LARGE_FILES
 	ifneq ($(shell expr "$(uname_V)" : '[1234]'),1)
 		THREADED_DELTA_SEARCH = YesPlease
+		THREADED_GREP = YesPlease
 	else
 		NO_PTHREADS = YesPlease
 	endif
@@ -916,6 +927,7 @@ ifeq ($(uname_S),IRIX)
 	SHELL_PATH = /usr/gnu/bin/bash
 	NEEDS_LIBGEN = YesPlease
 	THREADED_DELTA_SEARCH = YesPlease
+	THREADED_GREP = YesPlease
 endif
 ifeq ($(uname_S),IRIX64)
 	NO_SETENV=YesPlease
@@ -935,6 +947,7 @@ ifeq ($(uname_S),IRIX64)
 	SHELL_PATH=/usr/gnu/bin/bash
 	NEEDS_LIBGEN = YesPlease
 	THREADED_DELTA_SEARCH = YesPlease
+	THREADED_GREP = YesPlease
 endif
 ifeq ($(uname_S),HP-UX)
 	NO_IPV6=YesPlease
@@ -1334,6 +1347,7 @@ endif

 ifdef NO_PTHREADS
 	THREADED_DELTA_SEARCH =
+	THREADED_GREP =
 	BASIC_CFLAGS += -DNO_PTHREADS
 else
 	EXTLIBS += $(PTHREAD_LIBS)
@@ -1341,6 +1355,11 @@ endif

 ifdef THREADED_DELTA_SEARCH
 	BASIC_CFLAGS += -DTHREADED_DELTA_SEARCH
+endif
+ifdef THREADED_GREP
+	BASIC_CFLAGS += -DTHREADED_GREP
+endif
+ifneq (,$(THREADED_GREP)$(THREADED_DELTA_SEARCH))
 	LIB_OBJS += thread-utils.o
 endif
 ifdef DIR_HAS_BSD_GROUP_SEMANTICS
diff --git a/builtin-grep.c b/builtin-grep.c
index 9bd467c..911d0da 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c
@@ -16,7 +16,7 @@
 #include "quote.h"
 #include "dir.h"

-#ifndef NO_PTHREADS
+#ifdef THREADED_GREP
 #include "thread-utils.h"
 #include <pthread.h>
 #endif
@@ -28,7 +28,7 @@ static char const * const grep_usage[] = {

 static int use_threads = 1;

-#ifndef NO_PTHREADS
+#ifdef THREADED_GREP
 #define THREADS 8
 static pthread_t threads[THREADS];

@@ -274,7 +274,7 @@ static int wait_all(void)

 	return hit;
 }
-#else /* !NO_PTHREADS */
+#else /* THREADED_GREP */
 #define read_sha1_lock()
 #define read_sha1_unlock()

@@ -439,7 +439,7 @@ static int grep_sha1(struct grep_opt *opt, const unsigned char *sha1,

 	name = strbuf_detach(&pathbuf, NULL);

-#ifndef NO_PTHREADS
+#ifdef THREADED_GREP
 	if (use_threads) {
 		grep_sha1_async(opt, name, sha1);
 		return 0;
@@ -501,7 +501,7 @@ static int grep_file(struct grep_opt *opt, const char *filename)
 		strbuf_addstr(&buf, filename);
 	name = strbuf_detach(&buf, NULL);

-#ifndef NO_PTHREADS
+#ifdef THREADED_GREP
 	if (use_threads) {
 		grep_file_async(opt, name, filename);
 		return 0;
@@ -902,7 +902,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 	if ((opt.regflags != REG_NEWLINE) && opt.fixed)
 		die("cannot mix --fixed-strings and regexp");

-#ifndef NO_PTHREADS
+#ifdef THREADED_GREP
 	if (online_cpus() == 1 || !grep_threads_ok(&opt))
 		use_threads = 0;

-- 
1.7.0.rc0.1096.g3300c.dirty

[RFC/PATCH] MSVC: Windows-native implementation of pthread_cond_broadcast

[Zoltán Füzesi]

Threaded grep feature relies on this function, but its implementation was
missing from the Win32 Pthreads API.

Signed-off-by: Zoltán Füzesi <zfuzesi@eaglet.hu>
---
Johannes, how about this? I haven't tested it.

 compat/win32/pthread.c |   12 ++++++++++++
 compat/win32/pthread.h |    2 ++
 2 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/compat/win32/pthread.c b/compat/win32/pthread.c
index 631c0a4..498b552 100644
--- a/compat/win32/pthread.c
+++ b/compat/win32/pthread.c
@@ -108,3 +108,15 @@ int pthread_cond_signal(pthread_cond_t *cond)
 	else
 		return 0;
 }
+
+int pthread_cond_broadcast(pthread_cond_t *cond)
+{
+	LONG prev_count;
+	while (cond->waiters) {
+		if (!ReleaseSemaphore(cond->sema, 1, &prev_count))
+			return err_win_to_posix(GetLastError());
+		if (prev_count == LONG_MAX - 1)
+			break;
+	}
+	return 0;
+}
diff --git a/compat/win32/pthread.h b/compat/win32/pthread.h
index b8e1bcb..11426f5 100644
--- a/compat/win32/pthread.h
+++ b/compat/win32/pthread.h
@@ -44,6 +44,8 @@ extern int pthread_cond_wait(pthread_cond_t *cond, CRITICAL_SECTION *mutex);
 
 extern int pthread_cond_signal(pthread_cond_t *cond);
 
+extern int pthread_cond_broadcast(pthread_cond_t *cond);
+
 /*
  * Simple thread creation implementation using pthread API
  */
-- 
1.7.0.rc0.48.gdace5.dirty

Re: [RFC/PATCH] MSVC: Windows-native implementation of pthread_cond_broadcast

[Johannes Sixt]

On Freitag, 29. Januar 2010, Zoltán Füzesi wrote:
> Johannes, how about this? I haven't tested it.

Neither have I, but I do not expect that it work: You neither modify the 
waiters count nor do you wait for it to be modified; IOW, you have a busy 
loop. cond_broadcast is not that trivial. I hope to send a patch tonight.

-- Hannes

[PATCH] Windows: a minimal pthread_cond_broadcast

[Johannes Sixt]

[Cc msysgit list]

On Freitag, 29. Januar 2010, Johannes Sixt wrote:
> cond_broadcast is not that trivial.

... except when it can be tailor-made for a particular use-case.
What do people think about this?

I had implemented a full-blown pthread_cond_broadcast, taking ACE as
an example, but I had doubts about its correctness. Then I noticed that
we do not need a complete implementation anyway. So here we go...

--- 8< ---
From: Johannes Sixt <j6t@kdbg.org>
Subject: [PATCH] Windows: a minimal pthread_cond_broadcast

This is not an implementation, but more a fake of pthread_cond_broadcast.
It is sufficient for the only call site in builtin-grep.c that we have
at this time. It works because the threads that wake up due to this call
do not call pthread_cond_wait anymore and terminate.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 compat/win32/pthread.c |   10 ++++++++++
 compat/win32/pthread.h |    4 +---
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/compat/win32/pthread.c b/compat/win32/pthread.c
index 631c0a4..7b5cac1 100644
--- a/compat/win32/pthread.c
+++ b/compat/win32/pthread.c
@@ -108,3 +108,13 @@ int pthread_cond_signal(pthread_cond_t *cond)
 	else
 		return 0;
 }
+
+/*
+ * FIXME: This is a fake implementation that is tailored for the only
+ * user that we currently have.
+ */
+int pthread_cond_broadcast(pthread_cond_t *cond)
+{
+	ReleaseSemaphore(cond->sema, cond->waiters, NULL);
+	return 0;
+}
diff --git a/compat/win32/pthread.h b/compat/win32/pthread.h
index b8e1bcb..7c360d5 100644
--- a/compat/win32/pthread.h
+++ b/compat/win32/pthread.h
@@ -37,12 +37,10 @@ typedef struct {
 } pthread_cond_t;
 
 extern int pthread_cond_init(pthread_cond_t *cond, const void *unused);
-
 extern int pthread_cond_destroy(pthread_cond_t *cond);
-
 extern int pthread_cond_wait(pthread_cond_t *cond, CRITICAL_SECTION *mutex);
-
 extern int pthread_cond_signal(pthread_cond_t *cond);
+extern int pthread_cond_broadcast(pthread_cond_t *cond);
 
 /*
  * Simple thread creation implementation using pthread API
-- 
1.6.6.264.ga6155

[PATCH] Implement pthread_cond_broadcast on Windows

[Johannes Sixt]

See http://www.cse.wustl.edu/~schmidt/win32-cv-1.html, section "The
SignalObjectAndWait solution". But note that this implementation does not
use SignalObjectAndWait (which is needed to achieve fairness, but we do
not need fairness).

Note that our implementations of pthread_cond_broadcast and
pthread_cond_signal require that they are invoked with the mutex held that
is used in the pthread_cond_wait calls.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 Junio,

 please queue this patch for 1.7.0-rc1 even though it has not undergone
 a lot of review - the result is better than a git that does not even
 build on Windows.

 I had another look at this complete implementation of p_c_b and am now
 convinced that it is correct, but an extra set of eye-balls is always
 appreciated. I've Cc'd people who had shown interest in pthreads on
 Windows in the past.

 -- Hannes

 compat/win32/pthread.c |  100 ++++++++++++++++++++++++++++++++++++++++++-----
 compat/win32/pthread.h |    9 ++--
 2 files changed, 94 insertions(+), 15 deletions(-)

diff --git a/compat/win32/pthread.c b/compat/win32/pthread.c
index 631c0a4..e592084 100644
--- a/compat/win32/pthread.c
+++ b/compat/win32/pthread.c
@@ -52,24 +52,38 @@ int win32_pthread_join(pthread_t *thread, void **value_ptr)
 int pthread_cond_init(pthread_cond_t *cond, const void *unused)
 {
 	cond->waiters = 0;
+	cond->was_broadcast = 0;
+	InitializeCriticalSection(&cond->waiters_lock);
 
 	cond->sema = CreateSemaphore(NULL, 0, LONG_MAX, NULL);
 	if (!cond->sema)
 		die("CreateSemaphore() failed");
+
+	cond->continue_broadcast = CreateEvent(NULL,	/* security */
+				FALSE,			/* auto-reset */
+				FALSE,			/* not signaled */
+				NULL);			/* name */
+	if (!cond->continue_broadcast)
+		die("CreateEvent() failed");
+
 	return 0;
 }
 
 int pthread_cond_destroy(pthread_cond_t *cond)
 {
 	CloseHandle(cond->sema);
-	cond->sema = NULL;
-
+	CloseHandle(cond->continue_broadcast);
+	DeleteCriticalSection(&cond->waiters_lock);
 	return 0;
 }
 
 int pthread_cond_wait(pthread_cond_t *cond, CRITICAL_SECTION *mutex)
 {
-	InterlockedIncrement(&cond->waiters);
+	int last_waiter;
+
+	EnterCriticalSection(&cond->waiters_lock);
+	cond->waiters++;
+	LeaveCriticalSection(&cond->waiters_lock);
 
 	/*
 	 * Unlock external mutex and wait for signal.
@@ -82,22 +96,52 @@ int pthread_cond_wait(pthread_cond_t *cond, CRITICAL_SECTION *mutex)
 	/* let's wait - ignore return value */
 	WaitForSingleObject(cond->sema, INFINITE);
 
-	/* we're done waiting, so make sure we decrease waiters count */
-	InterlockedDecrement(&cond->waiters);
-
+	/*
+	 * Decrease waiters count. If we are the last waiter, then we must
+	 * notify the broadcasting thread that it can continue.
+	 * But if we continued due to cond_signal, we do not have to do that
+	 * because the signaling thread knows that only one waiter continued.
+	 */
+	EnterCriticalSection(&cond->waiters_lock);
+	cond->waiters--;
+	last_waiter = cond->was_broadcast && cond->waiters == 0;
+	LeaveCriticalSection(&cond->waiters_lock);
+
+	if (last_waiter) {
+		/*
+		 * cond_broadcast was issued while mutex was held. This means
+		 * that all other waiters have continued, but are contending
+		 * for the mutex at the end of this function because the
+		 * broadcasting thread did not leave cond_broadcast, yet.
+		 * (This is so that it can be sure that each waiter has
+		 * consumed exactly one slice of the semaphor.)
+		 * The last waiter must tell the broadcasting thread that it
+		 * can go on.
+		 */
+		SetEvent(cond->continue_broadcast);
+		/*
+		 * Now we go on to contend with all other waiters for
+		 * the mutex. Auf in den Kampf!
+		 */
+	}
 	/* lock external mutex again */
 	EnterCriticalSection(mutex);
 
 	return 0;
 }
 
+/*
+ * IMPORTANT: This implementation requires that pthread_cond_signal
+ * is called while the mutex is held that is used in the corresponding
+ * pthread_cond_wait calls!
+ */
 int pthread_cond_signal(pthread_cond_t *cond)
 {
-	/*
-	 * Access to waiters count is atomic; see "Interlocked Variable Access"
-	 * http://msdn.microsoft.com/en-us/library/ms684122(VS.85).aspx
-	 */
-	int have_waiters = cond->waiters > 0;
+	int have_waiters;
+	
+	EnterCriticalSection(&cond->waiters_lock);
+	have_waiters = cond->waiters > 0;
+	LeaveCriticalSection(&cond->waiters_lock);
 
 	/*
 	 * Signal only when there are waiters
@@ -108,3 +152,37 @@ int pthread_cond_signal(pthread_cond_t *cond)
 	else
 		return 0;
 }
+
+/*
+ * DOUBLY IMPORTANT: This implementation requires that pthread_cond_broadcast
+ * is called while the mutex is held that is used in the corresponding
+ * pthread_cond_wait calls!
+ */
+int pthread_cond_broadcast(pthread_cond_t *cond)
+{
+	EnterCriticalSection(&cond->waiters_lock);
+
+	if ((cond->was_broadcast = cond->waiters > 0)) {
+		/* wake up all waiters */
+		ReleaseSemaphore(cond->sema, cond->waiters, NULL);
+		LeaveCriticalSection(&cond->waiters_lock);
+		/*
+		 * At this point all waiters continue. Each one takes its
+		 * slice of the semaphor. Now it's our turn to wait: Since
+		 * the external mutex is held, no thread can leave cond_wait,
+		 * yet. For this reason, we can be sure that no thread gets
+		 * a chance to eat *more* than one slice. OTOH, it means
+		 * that the last waiter must send us a wake-up.
+		 */
+		WaitForSingleObject(cond->continue_broadcast, INFINITE);
+		/*
+		 * Since the external mutex is held, no thread can enter
+		 * cond_wait, and, hence, it is safe to reset this flag
+		 * without cond->waiters_lock held.
+		 */
+		cond->was_broadcast = 0;
+	} else {
+		LeaveCriticalSection(&cond->waiters_lock);
+	}
+	return 0;
+}
diff --git a/compat/win32/pthread.h b/compat/win32/pthread.h
index b8e1bcb..c72f100 100644
--- a/compat/win32/pthread.h
+++ b/compat/win32/pthread.h
@@ -32,17 +32,18 @@
  * See also: http://www.cse.wustl.edu/~schmidt/win32-cv-1.html
  */
 typedef struct {
-	volatile LONG waiters;
+	LONG waiters;
+	int was_broadcast;
+	CRITICAL_SECTION waiters_lock;
 	HANDLE sema;
+	HANDLE continue_broadcast;
 } pthread_cond_t;
 
 extern int pthread_cond_init(pthread_cond_t *cond, const void *unused);
-
 extern int pthread_cond_destroy(pthread_cond_t *cond);
-
 extern int pthread_cond_wait(pthread_cond_t *cond, CRITICAL_SECTION *mutex);
-
 extern int pthread_cond_signal(pthread_cond_t *cond);
+extern int pthread_cond_broadcast(pthread_cond_t *cond);
 
 /*
  * Simple thread creation implementation using pthread API
-- 
1.6.6.264.ga6155

Re: [PATCH] Implement pthread_cond_broadcast on Windows

[Johannes Schindelin]

Hi,

On Sat, 30 Jan 2010, Johannes Sixt wrote:

> See http://www.cse.wustl.edu/~schmidt/win32-cv-1.html, section "The
> SignalObjectAndWait solution". But note that this implementation does not
> use SignalObjectAndWait (which is needed to achieve fairness, but we do
> not need fairness).
> 
> Note that our implementations of pthread_cond_broadcast and
> pthread_cond_signal require that they are invoked with the mutex held that
> is used in the pthread_cond_wait calls.
> 
> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
> ---
>  Junio,
> 
>  please queue this patch for 1.7.0-rc1 even though it has not undergone
>  a lot of review - the result is better than a git that does not even
>  build on Windows.

As you Cc:ed the msysGit list, do you want this in 4msysgit.git, too?

Ciao,
Dscho

Re: [PATCH] Implement pthread_cond_broadcast on Windows

[Johannes Sixt]

On Samstag, 30. Januar 2010, Johannes Schindelin wrote:
> Hi,
>
> On Sat, 30 Jan 2010, Johannes Sixt wrote:
> > See http://www.cse.wustl.edu/~schmidt/win32-cv-1.html, section "The
> > SignalObjectAndWait solution". But note that this implementation does not
> > use SignalObjectAndWait (which is needed to achieve fairness, but we do
> > not need fairness).
> >
> > Note that our implementations of pthread_cond_broadcast and
> > pthread_cond_signal require that they are invoked with the mutex held
> > that is used in the pthread_cond_wait calls.
> >
> > Signed-off-by: Johannes Sixt <j6t@kdbg.org>
> > ---
> >  Junio,
> >
> >  please queue this patch for 1.7.0-rc1 even though it has not undergone
> >  a lot of review - the result is better than a git that does not even
> >  build on Windows.
>
> As you Cc:ed the msysGit list, do you want this in 4msysgit.git, too?

I hope some clever people on the msysgit list can say aye or nay to this 
patch. If you intend to release an msysgit that contains 5b594f4 (Threaded 
grep), then you need *something*, otherwise the build fails. Whether you get 
that something via upstream or apply it directly is of course secondary, but 
I prefer the former. ;)

-- Hannes

Re: [PATCH] Implement pthread_cond_broadcast on Windows

[Johannes Schindelin]

Hi,

On Sat, 30 Jan 2010, Johannes Sixt wrote:

> On Samstag, 30. Januar 2010, Johannes Schindelin wrote:
>
> > On Sat, 30 Jan 2010, Johannes Sixt wrote:
> > > See http://www.cse.wustl.edu/~schmidt/win32-cv-1.html, section "The 
> > > SignalObjectAndWait solution". But note that this implementation 
> > > does not use SignalObjectAndWait (which is needed to achieve 
> > > fairness, but we do not need fairness).
> > >
> > > Note that our implementations of pthread_cond_broadcast and 
> > > pthread_cond_signal require that they are invoked with the mutex 
> > > held that is used in the pthread_cond_wait calls.
> > >
> > > Signed-off-by: Johannes Sixt <j6t@kdbg.org>
> > > ---
> > >  Junio,
> > >
> > >  please queue this patch for 1.7.0-rc1 even though it has not 
> > >  undergone a lot of review - the result is better than a git that 
> > >  does not even build on Windows.
> >
> > As you Cc:ed the msysGit list, do you want this in 4msysgit.git, too?
> 
> I hope some clever people on the msysgit list can say aye or nay to this 
> patch. If you intend to release an msysgit that contains 5b594f4 
> (Threaded grep), then you need *something*, otherwise the build fails. 
> Whether you get that something via upstream or apply it directly is of 
> course secondary, but I prefer the former. ;)

Okay, I will just merge mingw.git's master, then, once you say that the 
fix is in.

Ciao,
Dscho

Re: [RFC/PATCH] MSVC: Windows-native implementation of pthread_cond_broadcast

[Johannes Schindelin]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 437 bytes --]

Hi,

On Fri, 29 Jan 2010, Zoltán Füzesi wrote:

> Johannes, how about this?

Do you really mean me?

> I haven't tested it.

Then why the hell do you address me????

Seriously again, please test first before you post this. Once you have 
real proof that it works as you intend it, I will be _happy_ to have a 
look and most likely merge it into 4msysgit.git.  Before you tested it, I 
will be just annoyed.

Thanks for your work,
Dscho