Re: [PATCH] fix threaded grep for machines with only one cpu

Hi,

On Tue, 16 Feb 2010, Junio C Hamano wrote:

Johannes Schindelin [off-list ref] writes:

quoted

quoted

diff --git a/builtin-grep.c b/builtin-grep.c
index 26d4deb..5c1545e 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c

@@ -81,8 +81,8 @@ static pthread_mutex_t read_sha1_mutex;
 
 #define grep_lock() pthread_mutex_lock(&grep_mutex)
 #define grep_unlock() pthread_mutex_unlock(&grep_mutex)
-#define read_sha1_lock() pthread_mutex_lock(&read_sha1_mutex)
-#define read_sha1_unlock() pthread_mutex_unlock(&read_sha1_mutex)
+#define read_sha1_lock() do { if (use_threads) pthread_mutex_lock(&read_sha1_mutex); } while (0)
+#define read_sha1_unlock() do { if (use_threads) pthread_mutex_unlock(&read_sha1_mutex); } while (0)

This is inconsistent. Just look at the code above and tell me why it is so 
different.

It is because grep_mutex is protected by "use_threads" very high in the
callchain and do not need nor want extra if().

But I think this is much cleaner.  The patch replaces the one you are
replying to (i.e. read_sha1_{lock,unlock}() are unconditional).

-- >8 --
Subject: Fix use of mutex in threaded grep

The program can decide at runtime not to use threading even if the support
is compiled in.  In such a case, mutexes are not necessary and left
uninitialized.  But the code incorrectly tried to take and release the
read_sha1_mutex unconditionally.

Signed-off-by: Junio C Hamano <redacted>
Acked-by: Fredrik Kuivinen <redacted>
---

Yes, this one looks much, much nicer.

Ciao,
Dscho