{"thread":{"id":"26015","subject":"[PATCH] transport-helper: avoid dependency on thread-utils.h","startedAt":"2010-12-10T11:48:39Z","lastAt":"2010-12-10T19:51:28Z","messageCount":6,"participants":["Jonathan Nieder","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"157812","messageId":"20101210114839.GA5771@burratino","threadId":"26015","inReplyTo":null,"subject":"[PATCH] transport-helper: avoid dependency on thread-utils.h","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-12-10T11:48:39Z","receivedAt":"2010-12-10T11:48:39Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Since the transport-helper does not use recursive mutexes or ask the\nnumber of cores, it does not require any declarations from the\nthread-utils lib.  Removing the unnecessary #include avoids false\npositives from \"make CHECK_HEADER_DEPENDENCIES=1\".\n\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\nThe #include directive in question was added in 7851b1e60\n(remote-fd/ext: finishing touches after code review, 2010-11-17),\nat the same times as the necessary #include for <pthread.h>.\nThis patch seems to work for me, but I wouldn't be surprised if\nto be missing something.\n\n transport-helper.c |    1 -\n 1 files changed, 0 insertions(+), 1 deletions(-)\n\ndiff --git a/transport-helper.c b/transport-helper.c\nindex 3a50856..7c99051 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -11,7 +11,6 @@\n \n #ifndef NO_PTHREADS\n #include <pthread.h>\n-#include \"thread-utils.h\"\n #endif\n \n static int debug;\n-- \n1.7.2.4\n"},{"id":"157830","messageId":"7vhbeleber.fsf@alter.siamese.dyndns.org","threadId":"26015","inReplyTo":"20101210114839.GA5771@burratino","subject":"Re: [PATCH] transport-helper: avoid dependency on thread-utils.h","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-12-10T18:21:48Z","receivedAt":"2010-12-10T18:21:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> Since the transport-helper does not use recursive mutexes or ask the\n> number of cores, it does not require any declarations from the\n> thread-utils lib.  Removing the unnecessary #include avoids false\n> positives from \"make CHECK_HEADER_DEPENDENCIES=1\".\n\nSorry, I was slow and took me some time to realize what is going on.\n\nIsn't it more like \"Makefile does not say transport-helper.o depends on\nthread-utils.h, but transport-helper.c does include it\"?  IOW, it not a\nfalse positive; perhaps a real but an unnecessary positive.\n\n> The #include directive in question was added in 7851b1e60\n> (remote-fd/ext: finishing touches after code review, 2010-11-17),\n> at the same times as the necessary #include for <pthread.h>.\n\nI think the whole four-line block was cut & paste from other places that\nuse the threading, namely grep.c and pack-objects.c.\n\nI wonder if it is better to do something like this instead.  This way, we\ncan later add more things that threaded part of the system would need to\nthread-utils.h inside \"ifndef NO_PTHREADS\" and they will automatically get\nincluded by all the threaders.\n\n\n builtin/grep.c         |    4 ----\n builtin/pack-objects.c |    4 ----\n thread-utils.h         |    4 ++++\n transport-helper.c     |    4 ----\n 4 files changed, 4 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex adb5424..fdf7131 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -17,11 +17,7 @@\n #include \"grep.h\"\n #include \"quote.h\"\n #include \"dir.h\"\n-\n-#ifndef NO_PTHREADS\n-#include <pthread.h>\n #include \"thread-utils.h\"\n-#endif\n \n static char const * const grep_usage[] = {\n \t\"git grep [options] [-e] <pattern> [<rev>...] [[--] <path>...]\",\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex f027b3a..b0503b2 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -16,11 +16,7 @@\n #include \"list-objects.h\"\n #include \"progress.h\"\n #include \"refs.h\"\n-\n-#ifndef NO_PTHREADS\n-#include <pthread.h>\n #include \"thread-utils.h\"\n-#endif\n \n static const char pack_usage[] =\n   \"git pack-objects [ -q | --progress | --all-progress ]\\n\"\ndiff --git a/thread-utils.h b/thread-utils.h\nindex 1727a03..6fb98c3 100644\n--- a/thread-utils.h\n+++ b/thread-utils.h\n@@ -1,7 +1,11 @@\n #ifndef THREAD_COMPAT_H\n #define THREAD_COMPAT_H\n \n+#ifndef NO_PTHREADS\n+#include <pthread.h>\n+\n extern int online_cpus(void);\n extern int init_recursive_mutex(pthread_mutex_t*);\n \n+#endif\n #endif /* THREAD_COMPAT_H */\ndiff --git a/transport-helper.c b/transport-helper.c\nindex 3a50856..4e4754c 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -8,11 +8,7 @@\n #include \"quote.h\"\n #include \"remote.h\"\n #include \"string-list.h\"\n-\n-#ifndef NO_PTHREADS\n-#include <pthread.h>\n #include \"thread-utils.h\"\n-#endif\n \n static int debug;\n \n"},{"id":"157839","messageId":"20101210194637.GA6801@burratino","threadId":"26015","inReplyTo":"7vhbeleber.fsf@alter.siamese.dyndns.org","subject":"[PATCH 0/2] Re: transport-helper: avoid dependency on thread-utils.h","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-12-10T19:46:37Z","receivedAt":"2010-12-10T19:46:37Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n\n> I wonder if it is better to do something like this instead.\n\nYes, I think so.  How about this?\n\nJonathan Nieder (1):\n  Makefile: transport-helper uses thread-utils.h\n\nJunio C Hamano (1):\n  simplify inclusion of pthread.h\n\n Makefile               |    3 +--\n builtin/grep.c         |    4 ----\n builtin/pack-objects.c |    4 ----\n thread-utils.h         |    4 ++++\n transport-helper.c     |    4 ----\n 5 files changed, 5 insertions(+), 14 deletions(-)\n\n-- \n1.7.2.4\n"},{"id":"157840","messageId":"20101210194754.GB6801@burratino","threadId":"26015","inReplyTo":"20101210194637.GA6801@burratino","subject":"[PATCH 1/2] simplify inclusion of pthread.h","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-12-10T19:47:54Z","receivedAt":"2010-12-10T19:47:54Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"In v1.7.1.1~56^2 (Thread-safe xmalloc and xrealloc needs a recursive\nmutex, 2010-04-08), the thread-utils header started using\npthread_mutex_t, so callers, which generally had been using\n\n\t#ifndef NO_PTHREADS\n\t#include \"thread-utils.h\"\n\t#include <pthread.h>\n\t#endif\n\nbefore, were changed to include <pthread.h> first.  It is simpler to\nlet thread-utils.h take care of the whole matter.  This way, we can\nlater add more things that the threaded part of the system would need\nto thread-utils.h inside \"ifndef NO_PTHREADS\" and they will\nautomatically get included by all headers.\n\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\n builtin/grep.c         |    4 ----\n builtin/pack-objects.c |    4 ----\n thread-utils.h         |    4 ++++\n transport-helper.c     |    4 ----\n 4 files changed, 4 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex adb5424..fdf7131 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -17,11 +17,7 @@\n #include \"grep.h\"\n #include \"quote.h\"\n #include \"dir.h\"\n-\n-#ifndef NO_PTHREADS\n-#include <pthread.h>\n #include \"thread-utils.h\"\n-#endif\n \n static char const * const grep_usage[] = {\n \t\"git grep [options] [-e] <pattern> [<rev>...] [[--] <path>...]\",\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex f027b3a..b0503b2 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -16,11 +16,7 @@\n #include \"list-objects.h\"\n #include \"progress.h\"\n #include \"refs.h\"\n-\n-#ifndef NO_PTHREADS\n-#include <pthread.h>\n #include \"thread-utils.h\"\n-#endif\n \n static const char pack_usage[] =\n   \"git pack-objects [ -q | --progress | --all-progress ]\\n\"\ndiff --git a/thread-utils.h b/thread-utils.h\nindex 1727a03..6fb98c3 100644\n--- a/thread-utils.h\n+++ b/thread-utils.h\n@@ -1,7 +1,11 @@\n #ifndef THREAD_COMPAT_H\n #define THREAD_COMPAT_H\n \n+#ifndef NO_PTHREADS\n+#include <pthread.h>\n+\n extern int online_cpus(void);\n extern int init_recursive_mutex(pthread_mutex_t*);\n \n+#endif\n #endif /* THREAD_COMPAT_H */\ndiff --git a/transport-helper.c b/transport-helper.c\nindex 3a50856..4e4754c 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -8,11 +8,7 @@\n #include \"quote.h\"\n #include \"remote.h\"\n #include \"string-list.h\"\n-\n-#ifndef NO_PTHREADS\n-#include <pthread.h>\n #include \"thread-utils.h\"\n-#endif\n \n static int debug;\n \n-- \n1.7.2.4\n"},{"id":"157841","messageId":"20101210194825.GC6801@burratino","threadId":"26015","inReplyTo":"20101210194637.GA6801@burratino","subject":"[PATCH 2/2] Makefile: transport-helper uses thread-utils.h","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-12-10T19:48:25Z","receivedAt":"2010-12-10T19:48:25Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"transport-helper.o gained a dependency on thread-utils.h in\n7851b1e (remote-fd/ext: finishing touches after code review,\n2010-11-17).\n\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\n Makefile |    3 +--\n 1 files changed, 1 insertions(+), 2 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 1fe1a3b..41ab490 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1937,13 +1937,12 @@ builtin/branch.o builtin/checkout.o builtin/clone.o builtin/reset.o branch.o tra\n builtin/bundle.o bundle.o transport.o: bundle.h\n builtin/bisect--helper.o builtin/rev-list.o bisect.o: bisect.h\n builtin/clone.o builtin/fetch-pack.o transport.o: fetch-pack.h\n-builtin/grep.o: thread-utils.h\n+builtin/grep.o builtin/pack-objects.o transport-helper.o: thread-utils.h\n builtin/send-pack.o transport.o: send-pack.h\n builtin/log.o builtin/shortlog.o: shortlog.h\n builtin/prune.o builtin/reflog.o reachable.o: reachable.h\n builtin/commit.o builtin/revert.o wt-status.o: wt-status.h\n builtin/tar-tree.o archive-tar.o: tar.h\n-builtin/pack-objects.o: thread-utils.h\n connect.o transport.o http-backend.o: url.h\n http-fetch.o http-walker.o remote-curl.o transport.o walker.o: walker.h\n http.o http-walker.o http-push.o http-fetch.o remote-curl.o: http.h\n-- \n1.7.2.4\n"},{"id":"157842","messageId":"20101210195128.GD6801@burratino","threadId":"26015","inReplyTo":"20101210194754.GB6801@burratino","subject":"Re: [PATCH 1/2] simplify inclusion of pthread.h","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-12-10T19:51:28Z","receivedAt":"2010-12-10T19:51:28Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jonathan Nieder wrote:\n\n> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>\n\nThis one is of course\n\n\tFrom: Junio C Hamano <gitster@pobox.com>\n\nOften enough I remind myself to include the pseudo-header but when the\nrapid cutting-and-pasting time comes it is easy to forget. :/\n"}]}