Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes:
> I looked around a bit but ran out of time to identify the reason why
> this was not caught earlier.
>
> builtin/grep.c | 9 +++++----
> 1 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/grep.c b/builtin/grep.c
> index 92eeada..e94c5fe 100644
> --- a/builtin/grep.c
> +++ b/builtin/grep.c
> @@ -78,10 +78,11 @@ static pthread_mutex_t grep_mutex;
> /* Used to serialize calls to read_sha1_file. */
> 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 WHEN_THREADED(x) do { if (use_threads) (x); } while (0)
> +#define grep_lock() WHEN_THREADED(pthread_mutex_lock(&grep_mutex))
> +#define grep_unlock() WHEN_THREADED(pthread_mutex_unlock(&grep_mutex))
> +#define read_sha1_lock() WHEN_THREADED(pthread_mutex_lock(&read_sha1_mutex))
> +#define read_sha1_unlock() WHEN_THREADED(pthread_mutex_unlock(&read_sha1_mutex))
I think, from a quick glance, this is a good first step.
The remainder of this message are hints and random thoughts on potential
follow-up patches that may want to build on top of this patch for further
clean-ups (not specifically meant for Dscho but for other people on both
mailing lists).
- The patch makes the check for use_threads in lock_and_read_sha1_file()
redundant. The other user of read_sha1_lock/unlock in grep_object() can
take advantage of this change (see below).
- It makes me wonder if it is simpler to initialize mutexes even in
!use_threads case.
- Wouldn't the result be more readable to make these into static inline
functions?
- Could we lose "#ifndef NO_PTHREADS" inside grep_sha1(), grep_file(),
and possibly cmd_grep() functions and let the compiler optimize things
away under NO_PTHREADS compilation?
diff --git a/builtin/grep.c b/builtin/grep.c
index 7d0779f..60daa85 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -354,13 +354,9 @@ static void *lock_and_read_sha1_file(const unsigned char *sha1, enum object_type
{
void *data;
- if (use_threads) {
- read_sha1_lock();
- data = read_sha1_file(sha1, type, size);
- read_sha1_unlock();
- } else {
- data = read_sha1_file(sha1, type, size);
- }
+ read_sha1_lock();
+ data = read_sha1_file(sha1, type, size);
+ read_sha1_unlock();
return data;
}
--
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