Re: [PATCH] transport-helper: avoid dependency on thread-utils.h

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jonathan Nieder <jrnieder@xxxxxxxxx> writes:

> Since the transport-helper does not use recursive mutexes or ask the
> number of cores, it does not require any declarations from the
> thread-utils lib.  Removing the unnecessary #include avoids false
> positives from "make CHECK_HEADER_DEPENDENCIES=1".

Sorry, I was slow and took me some time to realize what is going on.

Isn't it more like "Makefile does not say transport-helper.o depends on
thread-utils.h, but transport-helper.c does include it"?  IOW, it not a
false positive; perhaps a real but an unnecessary positive.

> The #include directive in question was added in 7851b1e60
> (remote-fd/ext: finishing touches after code review, 2010-11-17),
> at the same times as the necessary #include for <pthread.h>.

I think the whole four-line block was cut & paste from other places that
use the threading, namely grep.c and pack-objects.c.

I wonder if it is better to do something like this instead.  This way, we
can later add more things that threaded part of the system would need to
thread-utils.h inside "ifndef NO_PTHREADS" and they will automatically get
included by all the threaders.


 builtin/grep.c         |    4 ----
 builtin/pack-objects.c |    4 ----
 thread-utils.h         |    4 ++++
 transport-helper.c     |    4 ----
 4 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/builtin/grep.c b/builtin/grep.c
index adb5424..fdf7131 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -17,11 +17,7 @@
 #include "grep.h"
 #include "quote.h"
 #include "dir.h"
-
-#ifndef NO_PTHREADS
-#include <pthread.h>
 #include "thread-utils.h"
-#endif
 
 static char const * const grep_usage[] = {
 	"git grep [options] [-e] <pattern> [<rev>...] [[--] <path>...]",
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index f027b3a..b0503b2 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -16,11 +16,7 @@
 #include "list-objects.h"
 #include "progress.h"
 #include "refs.h"
-
-#ifndef NO_PTHREADS
-#include <pthread.h>
 #include "thread-utils.h"
-#endif
 
 static const char pack_usage[] =
   "git pack-objects [ -q | --progress | --all-progress ]\n"
diff --git a/thread-utils.h b/thread-utils.h
index 1727a03..6fb98c3 100644
--- a/thread-utils.h
+++ b/thread-utils.h
@@ -1,7 +1,11 @@
 #ifndef THREAD_COMPAT_H
 #define THREAD_COMPAT_H
 
+#ifndef NO_PTHREADS
+#include <pthread.h>
+
 extern int online_cpus(void);
 extern int init_recursive_mutex(pthread_mutex_t*);
 
+#endif
 #endif /* THREAD_COMPAT_H */
diff --git a/transport-helper.c b/transport-helper.c
index 3a50856..4e4754c 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -8,11 +8,7 @@
 #include "quote.h"
 #include "remote.h"
 #include "string-list.h"
-
-#ifndef NO_PTHREADS
-#include <pthread.h>
 #include "thread-utils.h"
-#endif
 
 static int debug;
 

--
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


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]