{"thread":{"id":"22666","subject":"[PATCH] fix threaded grep for machines with only one cpu","startedAt":"2010-02-15T22:50:02Z","lastAt":"2010-02-18T09:10:42Z","messageCount":26,"participants":["Heiko Voigt","Johannes Schindelin","Junio C Hamano","Fredrik Kuivinen","Nicolas Pitre","Sverre Rabbelier","Paolo Bonzini"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"134671","messageId":"20100215225001.GA944@book.hvoigt.net","threadId":"22666","inReplyTo":null,"subject":"[PATCH] fix threaded grep for machines with only one cpu","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2010-02-15T22:50:02Z","receivedAt":"2010-02-15T22:50:02Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"In case the machine has only one cpu the initialization was\nskipped.\n\nSigned-off-by: Heiko Voigt \n---\n builtin-grep.c | 12 ++++++------\n 1 files changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin-grep.c b/builtin-grep.c\nindex 26d4deb..644051c 100644\n--- a/builtin-grep.c\n+++ b/builtin-grep.c\n@@ -220,12 +220,6 @@ static void start_threads(struct grep_opt *opt)\n {\n \tint i;\n \n-\tpthread_mutex_init(&grep_mutex, NULL);\n-\tpthread_mutex_init(&read_sha1_mutex, NULL);\n-\tpthread_cond_init(&cond_add, NULL);\n-\tpthread_cond_init(&cond_write, NULL);\n-\tpthread_cond_init(&cond_result, NULL);\n-\n \tfor (i = 0; i < ARRAY_SIZE(todo); i++) {\n \t\tstrbuf_init(&todo[i].out, 0);\n \t}\n@@ -880,6 +874,12 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \tif (online_cpus() == 1 || !grep_threads_ok(&opt))\n \t\tuse_threads = 0;\n \n+\tpthread_mutex_init(&grep_mutex, NULL);\n+\tpthread_mutex_init(&read_sha1_mutex, NULL);\n+\tpthread_cond_init(&cond_add, NULL);\n+\tpthread_cond_init(&cond_write, NULL);\n+\tpthread_cond_init(&cond_result, NULL);\n+\n \tif (use_threads)\n \t\tstart_threads(&opt);\n #else\n-- \n1.7.0.rc1.7.gc0da5\n"},{"id":"134673","messageId":"alpine.DEB.1.00.1002160008210.20986@pacific.mpi-cbg.de","threadId":"22666","inReplyTo":"20100215225001.GA944@book.hvoigt.net","subject":"Re: [PATCH] fix threaded grep for machines with only one cpu","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2010-02-15T23:09:38Z","receivedAt":"2010-02-15T23:09:38Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 15 Feb 2010, Heiko Voigt wrote:\n\n> In case the machine has only one cpu the initialization was\n> skipped.\n> \n> Signed-off-by: Heiko Voigt \n> ---\n\nThank you, Heiko, for fixing that nasty bug! It is already in 4msysgit's \n'devel' branch.\n\nCiao,\nDscho\n"},{"id":"134685","messageId":"7vwryet2cw.fsf@alter.siamese.dyndns.org","threadId":"22666","inReplyTo":"20100215225001.GA944@book.hvoigt.net","subject":"Re: [PATCH] fix threaded grep for machines with only one cpu","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-02-16T01:07:27Z","receivedAt":"2010-02-16T01:07:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Heiko Voigt writes:\n\n> In case the machine has only one cpu the initialization was\n> skipped.\n\nAn obvious question that forces reviewers to go \"Huh?\" is why this\nmatters.\n\nIf use_threads is false, we do not call start_threads(), and I think at\nleast the intent of the threaded grep code in 5b594f4 (Threaded grep,\n2010-01-25) is that uninitialized mutexes matter at all in that case.\n\nFor example, \"grep_mutex\" is touched by calling grep_lock() and\ngrep_unlock() macros, or pthread_* functions on it directly. There are\nfive functions that touch this mutex: add_work(), get_work(), work_done(),\nstart_threads(), wait_all().\n\n - add_work() is called from grep_sha1_async() and grep_file_async().\n They are both called _only_ when use_threads is true.\n\n - get_work() and work_done() are only called from run() and that is the\n function threads run from start_threads(). They cannot be reached\n unless use_threads is true.\n\n - wait_all() has three call sites in cmd_grep(); all of them trigger only\n when use_threads is true.\n\nSo at least for grep_mutex your patch shouldn't matter. Please explain\nuse of which mutex is broken and how your patch fixes it.\n\nI think the fix also assumes that an initialized mutex that is used but\nnot destroyed is not a programming error, as wait_all() is the ONLY\nfunction that destroys these mutexes and it is called ONLY when we are\nusing threads by calling start_threads(). But that assumption surely\nsmells like it is sweeping a bug under a rug rather than fixing it.\n\nIs there some programming rule I am not aware of, like \"if you define a\nmutex or cond, you must initialize it, even if you are not going to use it\nat all?\" If that is the case, I think your patch is necessary, but I\nsomehow find it a bit hard to believe.\n"},{"id":"134686","messageId":"7v3a12t24a.fsf@alter.siamese.dyndns.org","threadId":"22666","inReplyTo":"7vwryet2cw.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] fix threaded grep for machines with only one cpu","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-02-16T01:12:37Z","receivedAt":"2010-02-16T01:12:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Heiko Voigt writes:\n>\n>> In case the machine has only one cpu the initialization was\n>> skipped.\n>\n> An obvious question that forces reviewers to go \"Huh?\" is why this\n> matters.\n>\n> If use_threads is false, we do not call start_threads(), and I think at\n> least the intent of the threaded grep code in 5b594f4 (Threaded grep,\n> 2010-01-25) is that uninitialized mutexes matter at all in that case.\n\ns/matter/do not &/; obviously.\n"},{"id":"134692","messageId":"7vocjpnc5v.fsf@alter.siamese.dyndns.org","threadId":"22666","inReplyTo":"7vwryet2cw.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] fix threaded grep for machines with only one cpu","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-02-16T02:32:12Z","receivedAt":"2010-02-16T02:32:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> So at least for grep_mutex your patch shouldn't matter. Please explain\n> use of which mutex is broken and how your patch fixes it.\n\nIs this the answer to my question? With your patch, your single-threaded\nprogram will end up locking and unlocking this mutex needlessly, and the\nmutex is left undestroyed after you are done, I think.\n\n builtin-grep.c | 4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin-grep.c b/builtin-grep.c\nindex 26d4deb..5c1545e 100644\n--- a/builtin-grep.c\n+++ b/builtin-grep.c\n@@ -81,8 +81,8 @@ static pthread_mutex_t read_sha1_mutex;\n \n #define grep_lock() pthread_mutex_lock(&grep_mutex)\n #define grep_unlock() pthread_mutex_unlock(&grep_mutex)\n-#define read_sha1_lock() pthread_mutex_lock(&read_sha1_mutex)\n-#define read_sha1_unlock() pthread_mutex_unlock(&read_sha1_mutex)\n+#define read_sha1_lock() do { if (use_threads) pthread_mutex_lock(&read_sha1_mutex); } while (0)\n+#define read_sha1_unlock() do { if (use_threads) pthread_mutex_unlock(&read_sha1_mutex); } while (0)\n \n /* Signalled when a new work_item is added to todo. */\n static pthread_cond_t cond_add;\n"},{"id":"134693","messageId":"7vljetlx8r.fsf@alter.siamese.dyndns.org","threadId":"22666","inReplyTo":"7vocjpnc5v.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] fix threaded grep for machines with only one cpu","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-02-16T02:39:48Z","receivedAt":"2010-02-16T02:39:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Is this the answer to my question?\n\nIOW, please try this patch. I am planning to queue it to 'maint' as part\nof 1.7.0.1 if this is the right solution (which I obviously think it is).\n\n-- >8 --\nFrom: Junio C Hamano \nDate: Mon, 15 Feb 2010 18:34:28 -0800\nSubject: [PATCH] Fix use of mutex in threaded grep\n\nThe program can decide at runtime not to use threading even if the\nsupport is compiled in. In such a case, mutexes are not necessary\nand left uninitialized. But the code incorrectly tried to take and\nrelease the read_sha1_mutex unconditionally.\n\nSigned-off-by: Junio C Hamano \n---\n builtin-grep.c | 4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin-grep.c b/builtin-grep.c\nindex 26d4deb..5c1545e 100644\n--- a/builtin-grep.c\n+++ b/builtin-grep.c\n@@ -81,8 +81,8 @@ static pthread_mutex_t read_sha1_mutex;\n \n #define grep_lock() pthread_mutex_lock(&grep_mutex)\n #define grep_unlock() pthread_mutex_unlock(&grep_mutex)\n-#define read_sha1_lock() pthread_mutex_lock(&read_sha1_mutex)\n-#define read_sha1_unlock() pthread_mutex_unlock(&read_sha1_mutex)\n+#define read_sha1_lock() do { if (use_threads) pthread_mutex_lock(&read_sha1_mutex); } while (0)\n+#define read_sha1_unlock() do { if (use_threads) pthread_mutex_unlock(&read_sha1_mutex); } while (0)\n \n /* Signalled when a new work_item is added to todo. */\n static pthread_cond_t cond_add;\n-- \n1.7.0.17.g7e5eb\n"},{"id":"134715","messageId":"4c8ef71002152354s3be9503aja28e5426a735ca07@mail.gmail.com","threadId":"22666","inReplyTo":"7vljetlx8r.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] fix threaded grep for machines with only one cpu","fromName":"Fredrik Kuivinen","fromEmail":"frekui@gmail.com","sentAt":"2010-02-16T07:54:00Z","receivedAt":"2010-02-16T07:54:00Z","isPatch":true,"sender":{"key":"frekui@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13770967?v=4"},"body":"On Tue, Feb 16, 2010 at 03:39, Junio C Hamano wrote:\n> Junio C Hamano writes:\n>\n>> Is this the answer to my question?\n>\n> IOW, please try this patch.  I am planning to queue it to 'maint' as part\n> of 1.7.0.1 if this is the right solution (which I obviously think it is).\n>\n> -- >8 --\n> From: Junio C Hamano \n> Date: Mon, 15 Feb 2010 18:34:28 -0800\n> Subject: [PATCH] Fix use of mutex in threaded grep\n>\n> The program can decide at runtime not to use threading even if the\n> support is compiled in.  In such a case, mutexes are not necessary\n> and left uninitialized.  But the code incorrectly tried to take and\n> release the read_sha1_mutex unconditionally.\n>\n> Signed-off-by: Junio C Hamano \n> ---\n>  builtin-grep.c |    4 ++--\n>  1 files changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/builtin-grep.c b/builtin-grep.c\n> index 26d4deb..5c1545e 100644\n> --- a/builtin-grep.c\n> +++ b/builtin-grep.c\n> @@ -81,8 +81,8 @@ static pthread_mutex_t read_sha1_mutex;\n>\n>  #define grep_lock() pthread_mutex_lock(&grep_mutex)\n>  #define grep_unlock() pthread_mutex_unlock(&grep_mutex)\n> -#define read_sha1_lock() pthread_mutex_lock(&read_sha1_mutex)\n> -#define read_sha1_unlock() pthread_mutex_unlock(&read_sha1_mutex)\n> +#define read_sha1_lock() do { if (use_threads) pthread_mutex_lock(&read_sha1_mutex); } while (0)\n> +#define read_sha1_unlock() do { if (use_threads) pthread_mutex_unlock(&read_sha1_mutex); } while (0)\n>\n>  /* Signalled when a new work_item is added to todo. */\n>  static pthread_cond_t cond_add;\n\nThis is the correct fix. Thanks.\n\nAcked-by: Fredrik Kuivinen \n\n- Fredrik\n"},{"id":"134716","messageId":"alpine.DEB.1.00.1002160914140.20986@pacific.mpi-cbg.de","threadId":"22666","inReplyTo":"7vljetlx8r.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] fix threaded grep for machines with only one cpu","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2010-02-16T08:15:01Z","receivedAt":"2010-02-16T08:15:01Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 15 Feb 2010, Junio C Hamano wrote:\n\n> diff --git a/builtin-grep.c b/builtin-grep.c\n> index 26d4deb..5c1545e 100644\n> --- a/builtin-grep.c\n> +++ b/builtin-grep.c\n> @@ -81,8 +81,8 @@ static pthread_mutex_t read_sha1_mutex;\n> \n> #define grep_lock() pthread_mutex_lock(&grep_mutex)\n> #define grep_unlock() pthread_mutex_unlock(&grep_mutex)\n> -#define read_sha1_lock() pthread_mutex_lock(&read_sha1_mutex)\n> -#define read_sha1_unlock() pthread_mutex_unlock(&read_sha1_mutex)\n> +#define read_sha1_lock() do { if (use_threads) pthread_mutex_lock(&read_sha1_mutex); } while (0)\n> +#define read_sha1_unlock() do { if (use_threads) pthread_mutex_unlock(&read_sha1_mutex); } while (0)\n\nThis is inconsistent. Just look at the code above and tell me why it is so \ndifferent.\n\nCiao,\nDscho\n"},{"id":"134764","messageId":"20100216180209.GA1532@book.hvoigt.net","threadId":"22666","inReplyTo":"7vljetlx8r.fsf@alter.siamese.dyndns.org","subject":"Re: Re: [PATCH] fix threaded grep for machines with only one cpu","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2010-02-16T18:02:09Z","receivedAt":"2010-02-16T18:02:09Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"On Mon, Feb 15, 2010 at 06:39:48PM -0800, Junio C Hamano wrote:\n> Junio C Hamano writes:\n> \n> > Is this the answer to my question?\n\nSorry I forgot to add a reference to the thread from which the patch\noriginated: http://groups.google.com/group/msysgit/msg/5da53cf1ccf417cf\n\n> IOW, please try this patch. I am planning to queue it to 'maint' as part\n> of 1.7.0.1 if this is the right solution (which I obviously think it is).\n\nYes your patch does it correctly I just verified that the segfaults are\ngone as well. I think your solution is even nicer than mine. Thanks.\n\n> -- >8 --\n> From: Junio C Hamano \n> Date: Mon, 15 Feb 2010 18:34:28 -0800\n> Subject: [PATCH] Fix use of mutex in threaded grep\n[...]\n> diff --git a/builtin-grep.c b/builtin-grep.c\n> index 26d4deb..5c1545e 100644\n> --- a/builtin-grep.c\n> +++ b/builtin-grep.c\n> @@ -81,8 +81,8 @@ static pthread_mutex_t read_sha1_mutex;\n> \n> #define grep_lock() pthread_mutex_lock(&grep_mutex)\n> #define grep_unlock() pthread_mutex_unlock(&grep_mutex)\n> -#define read_sha1_lock() pthread_mutex_lock(&read_sha1_mutex)\n> -#define read_sha1_unlock() pthread_mutex_unlock(&read_sha1_mutex)\n> +#define read_sha1_lock() do { if (use_threads) pthread_mutex_lock(&read_sha1_mutex); } while (0)\n> +#define read_sha1_unlock() do { if (use_threads) pthread_mutex_unlock(&read_sha1_mutex); } while (0)\n\nOne minor thing: Would it not be even nicer having the while loop inside\nthe if clause? E.g like this\n\n#define read_sha1_lock() if (use_threads) do { pthread_mutex_lock(&read_sha1_mutex); } while (0)\n#define read_sha1_unlock() if (use_threads) do { pthread_mutex_unlock(&read_sha1_mutex); } while (0)\n\nIf the purpose was to force a thread switch it is not necessary when not\nusing threads.\n\ncheers Heiko\n"},{"id":"134765","messageId":"alpine.LFD.2.00.1002161352080.1946@xanadu.home","threadId":"22666","inReplyTo":"20100216180209.GA1532@book.hvoigt.net","subject":"Re: Re: [PATCH] fix threaded grep for machines with only one cpu","fromName":"Nicolas Pitre","fromEmail":"nico@fluxnic.net","sentAt":"2010-02-16T18:59:02Z","receivedAt":"2010-02-16T18:59:02Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Tue, 16 Feb 2010, Heiko Voigt wrote:\n\n> On Mon, Feb 15, 2010 at 06:39:48PM -0800, Junio C Hamano wrote:\n> > Junio C Hamano writes:\n> > \n> > > Is this the answer to my question?\n> \n> Sorry I forgot to add a reference to the thread from which the patch\n> originated: http://groups.google.com/group/msysgit/msg/5da53cf1ccf417cf\n> \n> > IOW, please try this patch. I am planning to queue it to 'maint' as part\n> > of 1.7.0.1 if this is the right solution (which I obviously think it is).\n> \n> Yes your patch does it correctly I just verified that the segfaults are\n> gone as well. I think your solution is even nicer than mine. Thanks.\n> \n> > -- >8 --\n> > From: Junio C Hamano \n> > Date: Mon, 15 Feb 2010 18:34:28 -0800\n> > Subject: [PATCH] Fix use of mutex in threaded grep\n> [...]\n> > diff --git a/builtin-grep.c b/builtin-grep.c\n> > index 26d4deb..5c1545e 100644\n> > --- a/builtin-grep.c\n> > +++ b/builtin-grep.c\n> > @@ -81,8 +81,8 @@ static pthread_mutex_t read_sha1_mutex;\n> > \n> > #define grep_lock() pthread_mutex_lock(&grep_mutex)\n> > #define grep_unlock() pthread_mutex_unlock(&grep_mutex)\n> > -#define read_sha1_lock() pthread_mutex_lock(&read_sha1_mutex)\n> > -#define read_sha1_unlock() pthread_mutex_unlock(&read_sha1_mutex)\n> > +#define read_sha1_lock() do { if (use_threads) pthread_mutex_lock(&read_sha1_mutex); } while (0)\n> > +#define read_sha1_unlock() do { if (use_threads) pthread_mutex_unlock(&read_sha1_mutex); } while (0)\n> \n> One minor thing: Would it not be even nicer having the while loop inside\n> the if clause? E.g like this\n> \n> #define read_sha1_lock() if (use_threads) do { pthread_mutex_lock(&read_sha1_mutex); } while (0)\n> #define read_sha1_unlock() if (use_threads) do { pthread_mutex_unlock(&read_sha1_mutex); } while (0)\n\nNo. Think what happens if you have code like this:\n\n\tif (foo == 1)\n\t\tread_sha1_lock();\n\telse\n\t\tbaz();\n\n\nNicolas\n"},{"id":"134766","messageId":"7vocjpng1w.fsf@alter.siamese.dyndns.org","threadId":"22666","inReplyTo":"20100216180209.GA1532@book.hvoigt.net","subject":"Re: [PATCH] fix threaded grep for machines with only one cpu","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-02-16T19:20:27Z","receivedAt":"2010-02-16T19:20:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Heiko Voigt writes:\n\n>> IOW, please try this patch. I am planning to queue it to 'maint' as part\n>> of 1.7.0.1 if this is the right solution (which I obviously think it is).\n>\n> Yes your patch does it correctly I just verified that the segfaults are\n> gone as well. I think your solution is even nicer than mine. Thanks.\n\n>> +#define read_sha1_lock() do { if (use_threads) pthread_mutex_lock(&read_sha1_mutex); } while (0)\n>> +#define read_sha1_unlock() do { if (use_threads) pthread_mutex_unlock(&read_sha1_mutex); } while (0)\n>\n> One minor thing: Would it not be even nicer having the while loop inside\n> the if clause? E.g like this\n>\n> #define read_sha1_lock() if (use_threads) do { pthread_mutex_lock(&read_sha1_mutex); } while (0)\n> #define read_sha1_unlock() if (use_threads) do { pthread_mutex_unlock(&read_sha1_mutex); } while (0)\n\nNo.\n\n\t#define frotz() do { this compound stmt; } while (0)\n\nis a common idiom to make a macro that expands to a compound stmt behave\nas if it is a simple function call to avoid bugs when it is expanded,\nregardless of in which context it is used by an unsuspecting caller.\n\nYour rewrite is pointless because it is the same as saying\n\n\t#define read_sha1_lock() if (use_threads) p_m_l(&r_s_m)\n\nand that is exactly what the idiom's use of \"do { } while (0)\" is all\nabout.\n\nTry this simple program.\n\n-- >8 ---- >8 ---- >8 ---- >8 ---- >8 ---- >8 ---- >8 ---- >8 --\n#include \n\n#define frotz() do { if (flag) printf(\"frotz\"); } while (0)\n#define xyzzy() if (flag) do { printf(\"xyzzy\"); } while (0)\n#define yomin() if (flag) printf(\"yomin\")\n\nvoid test(int foo, int bar, int baz, int flag)\n{\n if (foo)\n frotz();\n else if (bar)\n xyzzy();\n else if (baz)\n yomin();\n else\n printf(\"huh?\");\n\n printf(\"\\n\");\n}\n\nint main(int ac, char **av)\n{\n int foo, bar, baz;\n for (foo = 0; foo < 2; foo++)\n for (bar = 0; bar < 2; bar++)\n for (baz = 0; baz < 2; baz++) {\n printf(\"%d %d %d \", foo, bar, baz);\n test(foo, bar, baz, 1);\n }\n return 0;\n}\n-- >8 ---- >8 ---- >8 ---- >8 ---- >8 ---- >8 ---- >8 ---- >8 --\n\nTextually the \"test\" function expands to this:\n\n void test(int foo, int bar, int baz, int flag)\n {\n if (foo)\n do { if (flag) printf(\"frotz\"); } while (0);\n else if (bar)\n if (flag) do { printf(\"xyzzy\"); } while (0);\n else if (baz)\n if (flag) printf(\"yomin\");\n else\n printf(\"huh?\");\n\n printf(\"\\n\");\n }\n\nbut if you properly indent it, it looks like this:\n\n void test(int foo, int bar, int baz, int flag)\n {\n if (foo)\n do {\n if (flag)\n printf(\"frotz\");\n } while (0);\n else if (bar)\n if (flag)\n do {\n printf(\"xyzzy\");\n } while (0);\n else if (baz)\n if (flag)\n printf(\"yomin\");\n else\n printf(\"huh?\");\n printf(\"\\n\");\n }\n\nNotice how your version (xyzzy) broke the cascade of if..elseif..else.\n\nDon't they teach this in schools anymore?\n"},{"id":"134767","messageId":"fabb9a1e1002161126m2db5fe4i1572a9959665977f@mail.gmail.com","threadId":"22666","inReplyTo":"7vocjpng1w.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] fix threaded grep for machines with only one cpu","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2010-02-16T19:26:59Z","receivedAt":"2010-02-16T19:26:59Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Tue, Feb 16, 2010 at 20:20, Junio C Hamano wrote:\n> Don't they teach this in schools anymore?\n\nHave they ever? And no, they don't. At least in the Netherlands (and I\nsuspect it to be the same in other European countries, and perhaps\neven in the US) don't spend a whole lot of time teaching C. At my uni\nthey start out with Java, and stick with that most of the time. There\nare only brief (and often optional) ventures into other languages.\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"134783","messageId":"alpine.LFD.2.00.1002161458410.1946@xanadu.home","threadId":"22666","inReplyTo":"7vocjpng1w.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] fix threaded grep for machines with only one cpu","fromName":"Nicolas Pitre","fromEmail":"nico@fluxnic.net","sentAt":"2010-02-16T20:00:47Z","receivedAt":"2010-02-16T20:00:47Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Tue, 16 Feb 2010, Junio C Hamano wrote:\n\n[...]\n> Notice how your version (xyzzy) broke the cascade of if..elseif..else.\n> \n> Don't they teach this in schools anymore?\n\nWhat do you expect from academia? School and real life are still too \noften disconnected.\n\n\nNicolas\n"},{"id":"134784","messageId":"alpine.DEB.1.00.1002162118060.8642@intel-tinevez-2-302","threadId":"22666","inReplyTo":"alpine.LFD.2.00.1002161458410.1946@xanadu.home","subject":"Re: [PATCH] fix threaded grep for machines with only one cpu","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2010-02-16T20:19:05Z","receivedAt":"2010-02-16T20:19:05Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 16 Feb 2010, Nicolas Pitre wrote:\n\n> On Tue, 16 Feb 2010, Junio C Hamano wrote:\n> \n> [...]\n> > Notice how your version (xyzzy) broke the cascade of if..elseif..else.\n> > \n> > Don't they teach this in schools anymore?\n> \n> What do you expect from academia? School and real life are still too \n> often disconnected.\n\nActually, you know, I am quite happy that they do not teach _that_ \nparticular code template. It is enough that I have to suffer this ugliness \nfrom oldtimers, no need for newtimers piling onto this particular pile.\n\nCiao,\nDscho\n"},{"id":"134785","messageId":"alpine.LFD.2.00.1002161529411.1946@xanadu.home","threadId":"22666","inReplyTo":"alpine.DEB.1.00.1002162118060.8642@intel-tinevez-2-302","subject":"Re: [PATCH] fix threaded grep for machines with only one cpu","fromName":"Nicolas Pitre","fromEmail":"nico@fluxnic.net","sentAt":"2010-02-16T20:37:57Z","receivedAt":"2010-02-16T20:37:57Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Tue, 16 Feb 2010, Johannes Schindelin wrote:\n\n> Hi,\n> \n> On Tue, 16 Feb 2010, Nicolas Pitre wrote:\n> \n> > On Tue, 16 Feb 2010, Junio C Hamano wrote:\n> > \n> > [...]\n> > > Notice how your version (xyzzy) broke the cascade of if..elseif..else.\n> > > \n> > > Don't they teach this in schools anymore?\n> > \n> > What do you expect from academia? School and real life are still too \n> > often disconnected.\n> \n> Actually, you know, I am quite happy that they do not teach _that_ \n> particular code template. It is enough that I have to suffer this ugliness \n> from oldtimers, no need for newtimers piling onto this particular pile.\n\nUnfortunately the real world is not without its share of ugliness.\nAnd newtimers need to be better prepared to cope when they get loose.\n\nI guess that's one of the reasons I skipped many lectures at Uni...\nto hack on Linux 1.0.x instead.\n\n\nNicolas\n"},{"id":"134788","messageId":"20100216205732.GB1860@book.hvoigt.net","threadId":"22666","inReplyTo":"alpine.LFD.2.00.1002161458410.1946@xanadu.home","subject":"Re: Re: [PATCH] fix threaded grep for machines with only one cpu","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2010-02-16T20:57:33Z","receivedAt":"2010-02-16T20:57:33Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"On Tue, Feb 16, 2010 at 03:00:47PM -0500, Nicolas Pitre wrote:\n> On Tue, 16 Feb 2010, Junio C Hamano wrote:\n> \n> [...]\n> > Notice how your version (xyzzy) broke the cascade of if..elseif..else.\n> > \n> > Don't they teach this in schools anymore?\n> \n> What do you expect from academia? School and real life are still too \n> often disconnected.\n\nIf you want to offend people that try to help out in real life. This is\nexactly the kind of comment that does it.\n"},{"id":"134789","messageId":"alpine.LFD.2.00.1002161606570.1946@xanadu.home","threadId":"22666","inReplyTo":"20100216205732.GB1860@book.hvoigt.net","subject":"Re: Re: [PATCH] fix threaded grep for machines with only one cpu","fromName":"Nicolas Pitre","fromEmail":"nico@fluxnic.net","sentAt":"2010-02-16T21:20:14Z","receivedAt":"2010-02-16T21:20:14Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Tue, 16 Feb 2010, Heiko Voigt wrote:\n\n> On Tue, Feb 16, 2010 at 03:00:47PM -0500, Nicolas Pitre wrote:\n> > What do you expect from academia? School and real life are still too \n> > often disconnected.\n> \n> If you want to offend people that try to help out in real life. This is\n> exactly the kind of comment that does it.\n\nBecause you think that I'm not giving my own time helping people solving \nreal life issues? If so I'd suggest you do a quick background check on \nmyself.\n\nI do get offence, though, when theoricians try to tell me how to do the \nthings they never were able to do themselves because they get so blinded \nby concepts over practicalities. Dont get me wrong -- there are a good \nbunch of individuals with practical sense in academia, but they usually \naren't those who get most credits.\n\n\nNicolas\n"},{"id":"134791","messageId":"7v1vgk94gq.fsf@alter.siamese.dyndns.org","threadId":"22666","inReplyTo":"alpine.LFD.2.00.1002161606570.1946@xanadu.home","subject":"Re: [PATCH] fix threaded grep for machines with only one cpu","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-02-16T22:54:29Z","receivedAt":"2010-02-16T22:54:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nicolas Pitre writes:\n\n> On Tue, 16 Feb 2010, Heiko Voigt wrote:\n>\n>> On Tue, Feb 16, 2010 at 03:00:47PM -0500, Nicolas Pitre wrote:\n>> > What do you expect from academia? School and real life are still too \n>> > often disconnected.\n>> \n>> If you want to offend people that try to help out in real life. This is\n>> exactly the kind of comment that does it.\n>\n> Because you think that I'm not giving my own time helping people solving \n> real life issues? If so I'd suggest you do a quick background check on \n> myself.\n\nI don't think Heiko is saying he was offended by you, but is saying he was\noffended by me saying \"Don't they teach this in schools anymore?\" I was\nsimply curious about the answer to that question. If they don't teach C,\nit is not Heiko's fault---no need to get offended. I admit that I did go\n\"Huh? Never repeat? What's the point?\" when I first saw the idiom.\n\nBy learning and sticking to idioms people get more efficient, because\nthere is no need to think if it is better to have \"do { ... } while()\"\ninside or outside yourself; cleverer people have already figured out the\nbest way and that is how idioms came about. Once you learn them, your\neyes will coast over the outer cruft and focus on the essential part of\nthe macro \"...\" without even spending any extra effort to actively squelch\nthe surrounding \"do {\" and \"} while (0)\". These literally become part of\nbackground noise and do not bother you anymore to the point that they are\nnot even ugly.\n\nThink of this as a mutual/collective learning experience. As Dscho said\non this topic on msysgit list, Heiko _is_ the hero in figuring out what\nwas originally broken, even though the proposed solution might have been\nsuboptimal. I know figuring out what is broken have taken a lot more\neffort than my looking over his solution and reaching a better solution.\nCorrect diagnosis is often more than 80% of solving a problem.\n"},{"id":"134815","messageId":"7viq9wzq8g.fsf@alter.siamese.dyndns.org","threadId":"22666","inReplyTo":"alpine.DEB.1.00.1002160914140.20986@pacific.mpi-cbg.de","subject":"Re: [PATCH] fix threaded grep for machines with only one cpu","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-02-16T23:59:43Z","receivedAt":"2010-02-16T23:59:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n>> diff --git a/builtin-grep.c b/builtin-grep.c\n>> index 26d4deb..5c1545e 100644\n>> --- a/builtin-grep.c\n>> +++ b/builtin-grep.c\n>> @@ -81,8 +81,8 @@ static pthread_mutex_t read_sha1_mutex;\n>> \n>> #define grep_lock() pthread_mutex_lock(&grep_mutex)\n>> #define grep_unlock() pthread_mutex_unlock(&grep_mutex)\n>> -#define read_sha1_lock() pthread_mutex_lock(&read_sha1_mutex)\n>> -#define read_sha1_unlock() pthread_mutex_unlock(&read_sha1_mutex)\n>> +#define read_sha1_lock() do { if (use_threads) pthread_mutex_lock(&read_sha1_mutex); } while (0)\n>> +#define read_sha1_unlock() do { if (use_threads) pthread_mutex_unlock(&read_sha1_mutex); } while (0)\n>\n> This is inconsistent. Just look at the code above and tell me why it is so \n> different.\n\nIt is because grep_mutex is protected by \"use_threads\" very high in the\ncallchain and do not need nor want extra if().\n\nBut I think this is much cleaner. The patch replaces the one you are\nreplying to (i.e. read_sha1_{lock,unlock}() are unconditional).\n\n-- >8 --\nSubject: Fix use of mutex in threaded grep\n\nThe program can decide at runtime not to use threading even if the support\nis compiled in. In such a case, mutexes are not necessary and left\nuninitialized. But the code incorrectly tried to take and release the\nread_sha1_mutex unconditionally.\n\nSigned-off-by: Junio C Hamano \nAcked-by: Fredrik Kuivinen \n---\n\n builtin-grep.c | 25 ++++++++++++++++---------\n 1 files changed, 16 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin-grep.c b/builtin-grep.c\nindex 26d4deb..8cec8b6 100644\n--- a/builtin-grep.c\n+++ b/builtin-grep.c\n@@ -408,15 +408,25 @@ static int pathspec_matches(const char **paths, const char *name, int max_depth)\n \treturn 0;\n }\n \n+static void *read_sha1_file_locked(const unsigned char *sha1, enum object_type *type, unsigned long *size)\n+{\n+\tvoid *data;\n+\n+\tif (use_threads) {\n+\t\tread_sha1_lock();\n+\t\tdata = read_sha1_file(sha1, type, size);\n+\t\tread_sha1_unlock();\n+\t} else {\n+\t\tdata = read_sha1_file(sha1, type, size);\n+\t}\n+\treturn data;\n+}\n+\n static void *load_sha1(const unsigned char *sha1, unsigned long *size,\n \t\t const char *name)\n {\n \tenum object_type type;\n-\tchar *data;\n-\n-\tread_sha1_lock();\n-\tdata = read_sha1_file(sha1, &type, size);\n-\tread_sha1_unlock();\n+\tvoid *data = read_sha1_file_locked(sha1, &type, size);\n \n \tif (!data)\n \t\terror(\"'%s': unable to read %s\", name, sha1_to_hex(sha1));\n@@ -605,10 +615,7 @@ static int grep_tree(struct grep_opt *opt, const char **paths,\n \t\t\tvoid *data;\n \t\t\tunsigned long size;\n \n-\t\t\tread_sha1_lock();\n-\t\t\tdata = read_sha1_file(entry.sha1, &type, &size);\n-\t\t\tread_sha1_unlock();\n-\n+\t\t\tdata = read_sha1_file_locked(entry.sha1, &type, &size);\n \t\t\tif (!data)\n \t\t\t\tdie(\"unable to read tree (%s)\",\n \t\t\t\t sha1_to_hex(entry.sha1));\n"},{"id":"134824","messageId":"alpine.DEB.1.00.1002170200370.20986@pacific.mpi-cbg.de","threadId":"22666","inReplyTo":"7viq9wzq8g.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] fix threaded grep for machines with only one cpu","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2010-02-17T01:01:03Z","receivedAt":"2010-02-17T01:01:03Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 16 Feb 2010, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n> \n> >> diff --git a/builtin-grep.c b/builtin-grep.c\n> >> index 26d4deb..5c1545e 100644\n> >> --- a/builtin-grep.c\n> >> +++ b/builtin-grep.c\n> >> @@ -81,8 +81,8 @@ static pthread_mutex_t read_sha1_mutex;\n> >> \n> >> #define grep_lock() pthread_mutex_lock(&grep_mutex)\n> >> #define grep_unlock() pthread_mutex_unlock(&grep_mutex)\n> >> -#define read_sha1_lock() pthread_mutex_lock(&read_sha1_mutex)\n> >> -#define read_sha1_unlock() pthread_mutex_unlock(&read_sha1_mutex)\n> >> +#define read_sha1_lock() do { if (use_threads) pthread_mutex_lock(&read_sha1_mutex); } while (0)\n> >> +#define read_sha1_unlock() do { if (use_threads) pthread_mutex_unlock(&read_sha1_mutex); } while (0)\n> >\n> > This is inconsistent. Just look at the code above and tell me why it is so \n> > different.\n> \n> It is because grep_mutex is protected by \"use_threads\" very high in the\n> callchain and do not need nor want extra if().\n> \n> But I think this is much cleaner. The patch replaces the one you are\n> replying to (i.e. read_sha1_{lock,unlock}() are unconditional).\n> \n> -- >8 --\n> Subject: Fix use of mutex in threaded grep\n> \n> The program can decide at runtime not to use threading even if the support\n> is compiled in. In such a case, mutexes are not necessary and left\n> uninitialized. But the code incorrectly tried to take and release the\n> read_sha1_mutex unconditionally.\n> \n> Signed-off-by: Junio C Hamano \n> Acked-by: Fredrik Kuivinen \n> ---\n\nYes, this one looks much, much nicer.\n\nCiao,\nDscho\n"},{"id":"134828","messageId":"7vvddwodox.fsf@alter.siamese.dyndns.org","threadId":"22666","inReplyTo":"alpine.DEB.1.00.1002170200370.20986@pacific.mpi-cbg.de","subject":"Re: [PATCH] fix threaded grep for machines with only one cpu","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-02-17T01:26:06Z","receivedAt":"2010-02-17T01:26:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n>> The program can decide at runtime not to use threading even if the support\n>> is compiled in. In such a case, mutexes are not necessary and left\n>> uninitialized. But the code incorrectly tried to take and release the\n>> read_sha1_mutex unconditionally.\n>> \n>> Signed-off-by: Junio C Hamano \n>> Acked-by: Fredrik Kuivinen \n>> ---\n>\n> Yes, this one looks much, much nicer.\n\nThe structure may be much nicer, but one remaining thing is that I do not\nthink foo_locked() is a good name; IIRC, kernel folks use _locked() suffix\nwhen the caller is expected to already hold the lock. So a typical naming\nconvention goes like this:\n\n\tfoo()\n {\n \tlock();\n foo_locked();\n unlock();\n }\n\nbut what the patch did was the other way around:\n\n\tread_sha1_file_locked()\n {\n \tlock();\n read_sha1_file();\n unlock();\n\t}\n\nwhich is probably against the convention many readers of our codebase are\nalready familiar with.\n\nWe need a better name to unconfuse people, I think.\n"},{"id":"134831","messageId":"alpine.LFD.2.00.1002162140110.1946@xanadu.home","threadId":"22666","inReplyTo":"7vvddwodox.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] fix threaded grep for machines with only one cpu","fromName":"Nicolas Pitre","fromEmail":"nico@fluxnic.net","sentAt":"2010-02-17T02:41:20Z","receivedAt":"2010-02-17T02:41:20Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Tue, 16 Feb 2010, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n> \n> >> The program can decide at runtime not to use threading even if the support\n> >> is compiled in. In such a case, mutexes are not necessary and left\n> >> uninitialized. But the code incorrectly tried to take and release the\n> >> read_sha1_mutex unconditionally.\n> >> \n> >> Signed-off-by: Junio C Hamano \n> >> Acked-by: Fredrik Kuivinen \n> >> ---\n> >\n> > Yes, this one looks much, much nicer.\n> \n> The structure may be much nicer, but one remaining thing is that I do not\n> think foo_locked() is a good name; IIRC, kernel folks use _locked() suffix\n> when the caller is expected to already hold the lock. So a typical naming\n> convention goes like this:\n> \n> \tfoo()\n> {\n> \tlock();\n> foo_locked();\n> unlock();\n> }\n> \n> but what the patch did was the other way around:\n> \n> \tread_sha1_file_locked()\n> {\n> \tlock();\n> read_sha1_file();\n> unlock();\n> \t}\n> \n> which is probably against the convention many readers of our codebase are\n> already familiar with.\n> \n> We need a better name to unconfuse people, I think.\n\nlock_and_read_sha1_file()\n\n\nNicolas\n"},{"id":"134866","messageId":"4B7C0B4D.6080803@gnu.org","threadId":"22666","inReplyTo":"alpine.LFD.2.00.1002161458410.1946@xanadu.home","subject":"Re: [PATCH] fix threaded grep for machines with only one cpu","fromName":"Paolo Bonzini","fromEmail":"bonzini@gnu.org","sentAt":"2010-02-17T15:29:17Z","receivedAt":"2010-02-17T15:29:17Z","isPatch":true,"sender":{"key":"bonzini@gnu.org","avatar":"https://avatars.githubusercontent.com/u/42082?v=4"},"body":"On 02/16/2010 09:00 PM, Nicolas Pitre wrote:\n> On Tue, 16 Feb 2010, Junio C Hamano wrote:\n>\n> [...]\n>> Notice how your version (xyzzy) broke the cascade of if..elseif..else.\n>>\n>> Don't they teach this in schools anymore?\n>\n> What do you expect from academia? School and real life are still too\n> often disconnected.\n\nWhen I taught C to second-year bachelor students, I think I did a pretty \ngood course (*) but it totally lacked time to get into macros, except \nfor simple constants. I did have a student later on that was doing his \nfinal project with me and came asking what it was.\n\nAnother guy I know is teaching an elective \"portable programming\" course \nthat includes pretty much everything you'd expect (including \nbit-twiddling tricks, basic Autoconf, shared libraries, blah blah) but \nthat's a graduate-level course.\n\n (*) and not too disconnected from reality. One year their final\n one-week project was using cairo for graphics, had a server that\n talked to multiple clients using poll, and I forced them to support\n IPv6. Shameless plug: http://github.com/bonzini/netrobots\n\nPaolo\n"},{"id":"134870","messageId":"20100217175600.GB2251@book.hvoigt.net","threadId":"22666","inReplyTo":"7v1vgk94gq.fsf@alter.siamese.dyndns.org","subject":"Re: Re: [PATCH] fix threaded grep for machines with only one cpu","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2010-02-17T17:56:01Z","receivedAt":"2010-02-17T17:56:01Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"On Tue, Feb 16, 2010 at 02:54:29PM -0800, Junio C Hamano wrote:\n> Nicolas Pitre writes:\n> \n> > On Tue, 16 Feb 2010, Heiko Voigt wrote:\n> >\n> >> On Tue, Feb 16, 2010 at 03:00:47PM -0500, Nicolas Pitre wrote:\n> >> > What do you expect from academia? School and real life are still too \n> >> > often disconnected.\n> >> \n> >> If you want to offend people that try to help out in real life. This is\n> >> exactly the kind of comment that does it.\n> >\n> > Because you think that I'm not giving my own time helping people solving \n> > real life issues? If so I'd suggest you do a quick background check on \n> > myself.\n> \n> I don't think Heiko is saying he was offended by you, but is saying he was\n> offended by me saying \"Don't they teach this in schools anymore?\" I was\n> simply curious about the answer to that question. If they don't teach C,\n> it is not Heiko's fault---no need to get offended. I admit that I did go\n> \"Huh? Never repeat? What's the point?\" when I first saw the idiom.\n\nIt was actually the combination of both ;) But I am glad we could sort\nthis out. I simply had the same thought as you when I saw this first and\nthen confused it with the typical sleep(0) you use for forcing thread\nswitches.\n\nBTW, we had a course of C but for macros they stopped at \"type them\nuppercase and try to avoid them when possible\". Everything else is\nlearning by experience.\n\ncheers Heiko\n"},{"id":"134897","messageId":"20100217224330.GB8029@book.hvoigt.net","threadId":"22666","inReplyTo":"7viq9wzq8g.fsf@alter.siamese.dyndns.org","subject":"Re: Re: [PATCH] fix threaded grep for machines with only one cpu","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2010-02-17T22:43:30Z","receivedAt":"2010-02-17T22:43:30Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"On Tue, Feb 16, 2010 at 03:59:43PM -0800, Junio C Hamano wrote:\n> -- >8 --\n> Subject: Fix use of mutex in threaded grep\n> \n> The program can decide at runtime not to use threading even if the support\n> is compiled in. In such a case, mutexes are not necessary and left\n> uninitialized. But the code incorrectly tried to take and release the\n> read_sha1_mutex unconditionally.\n> \n> Signed-off-by: Junio C Hamano \n> Acked-by: Fredrik Kuivinen \n\nJust to be sure I just tested this one as well. If you like you can add\na:\n\nTested-by: Heiko Voigt \n\ncheers Heiko\n"},{"id":"134935","messageId":"alpine.DEB.1.00.1002181009440.20986@pacific.mpi-cbg.de","threadId":"22666","inReplyTo":"20100217224330.GB8029@book.hvoigt.net","subject":"Re: Re: [PATCH] fix threaded grep for machines with only one cpu","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2010-02-18T09:10:42Z","receivedAt":"2010-02-18T09:10:42Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 17 Feb 2010, Heiko Voigt wrote:\n\n> On Tue, Feb 16, 2010 at 03:59:43PM -0800, Junio C Hamano wrote:\n> > -- >8 --\n> > Subject: Fix use of mutex in threaded grep\n> > \n> > The program can decide at runtime not to use threading even if the support\n> > is compiled in. In such a case, mutexes are not necessary and left\n> > uninitialized. But the code incorrectly tried to take and release the\n> > read_sha1_mutex unconditionally.\n> > \n> > Signed-off-by: Junio C Hamano \n> > Acked-by: Fredrik Kuivinen \n> \n> Just to be sure I just tested this one as well. If you like you can add \n> a:\n> \n> Tested-by: Heiko Voigt \n\nI have pushed the commit (at least the second-last :-) to 4msysgit's devel \nalready (in the vain hope to release a Git for Windows on Tuesday...)\n\nCiao,\nDscho\n"}]}