Hi,
On Tue, 16 Feb 2010, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes:
>
> >> 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 <gitster@xxxxxxxxx>
> Acked-by: Fredrik Kuivinen <frekui@xxxxxxxxx>
> ---
Yes, this one looks much, much nicer.
Ciao,
Dscho
--
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