Re: [PATCH] git grep: be careful to use mutices only when they are initialized

From

Junio C Hamano <gitster@pobox.com>

Date

Oct 26, 2011, 20:08 UTC

Message-ID

<7vmxcn1pob.fsf@alter.siamese.dyndns.org>

In-Reply-To

<7v39ef34in.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> 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).
> ...
>  - 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?

But since I already did it,...

-- >8 --
Subject: [PATCH] builtin/grep: war on #if[n]def inside function body

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/grep.c |   37 +++++++++++++++++++------------------
 1 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/builtin/grep.c b/builtin/grep.c
index 3d7329d..f24f3a7 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -24,9 +24,13 @@ static char const * const grep_usage[] = {
 	NULL
 };
 
-static int use_threads = 1;
+/* Skip the leading hunk mark of the first file. */
+static int skip_first_line;
 
 #ifndef NO_PTHREADS
+static int use_threads = 1;
+#define no_threads() use_threads = 0
+
 #define THREADS 8
 static pthread_t threads[THREADS];
 
@@ -112,8 +116,6 @@ static pthread_cond_t cond_write;
 /* Signalled when we are finished with everything. */
 static pthread_cond_t cond_result;
 
-static int skip_first_line;
-
 static void add_work(enum work_type type, char *name, void *id)
 {
 	grep_lock();
@@ -181,7 +183,6 @@ static void work_done(struct work_item *w)
 			const char *p = w->out.buf;
 			size_t len = w->out.len;
 
-			/* Skip the leading hunk mark of the first file. */
 			if (skip_first_line) {
 				while (len) {
 					len--;
@@ -310,8 +311,18 @@ static int wait_all(void)
 	return hit;
 }
 #else /* !NO_PTHREADS */
+#define use_threads 0
+#define no_threads() /* noop */
+
 #define read_sha1_lock()
 #define read_sha1_unlock()
+#define grep_lock()
+#define grep_unlock()
+
+#define online_cpus() 1
+#define grep_sha1_async(opt, name, sha1)
+#define grep_file_async(opt, name, filename)
+#define start_threads(opt)
 
 static int wait_all(void)
 {
@@ -407,13 +418,10 @@ static int grep_sha1(struct grep_opt *opt, const unsigned char *sha1,
 
 	name = strbuf_detach(&pathbuf, NULL);
 
-#ifndef NO_PTHREADS
 	if (use_threads) {
 		grep_sha1_async(opt, name, sha1);
 		return 0;
-	} else
-#endif
-	{
+	} else {
 		int hit;
 		unsigned long sz;
 		void *data = load_sha1(sha1, &sz, name);
@@ -469,13 +477,10 @@ static int grep_file(struct grep_opt *opt, const char *filename)
 		strbuf_addstr(&buf, filename);
 	name = strbuf_detach(&buf, NULL);
 
-#ifndef NO_PTHREADS
 	if (use_threads) {
 		grep_file_async(opt, name, filename);
 		return 0;
-	} else
-#endif
-	{
+	} else {
 		int hit;
 		size_t sz;
 		void *data = load_file(filename, &sz);
@@ -992,7 +997,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 		opt.output_priv = &path_list;
 		opt.output = append_path;
 		string_list_append(&path_list, show_in_pager);
-		use_threads = 0;
+		no_threads();
 	}
 
 	if (!opt.pattern_list)
@@ -1000,9 +1005,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 	if (!opt.fixed && opt.ignore_case)
 		opt.regflags |= REG_ICASE;
 
-#ifndef NO_PTHREADS
 	if (online_cpus() == 1 || !grep_threads_ok(&opt))
-		use_threads = 0;
+		no_threads();
 
 	if (use_threads) {
 		if (opt.pre_context || opt.post_context || opt.file_break ||
@@ -1010,9 +1014,6 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 			skip_first_line = 1;
 		start_threads(&opt);
 	}
-#else
-	use_threads = 0;
-#endif
 
 	compile_grep_patterns(&opt);
 

-- 
1.7.7.1.504.gcc718