{"thread":{"id":"44135","subject":"[PATCH] run-command: async_exit no longer needs to be public","startedAt":"2016-09-22T16:56:30Z","lastAt":"2016-09-24T01:04:58Z","messageCount":6,"participants":["Ramsay Jones","Lars Schneider","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"302378","messageId":"78f2bdd0-f6ad-db5c-f9f2-f90528bc4f77@ramsayjones.plus.com","threadId":"44135","inReplyTo":null,"subject":"[PATCH] run-command: async_exit no longer needs to be public","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2016-09-22T16:56:05Z","receivedAt":"2016-09-22T16:56:30Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\nSigned-off-by: Ramsay Jones \n---\n\nHi Lars,\n\nIf you need to re-roll your 'ls/filter-process' branch, could you please\nsquash this into the relevant commit c42a4cbc (\"run-command: move check_pipe()\nfrom write_or_die to run_command\", 20-09-2016).\n\n[Note that commit 9658846c (\"write_or_die: handle EPIPE in async threads\",\n24-02-2016) introduced async_exit() specifically for use in the implementation\nof check_pipe(). Now that you have moved check_pipe() into run-command.c,\nit no longer needs to be public.]\n\nThanks!\n\nATB,\nRamsay Jones\n\n run-command.c | 30 +++++++++++++++---------------\n run-command.h | 3 +--\n 2 files changed, 16 insertions(+), 17 deletions(-)\n\ndiff --git a/run-command.c b/run-command.c\nindex b72f6d1..3269362 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -6,19 +6,6 @@\n #include \"thread-utils.h\"\n #include \"strbuf.h\"\n \n-void check_pipe(int err)\n-{\n-\tif (err == EPIPE) {\n-\t\tif (in_async())\n-\t\t\tasync_exit(141);\n-\n-\t\tsignal(SIGPIPE, SIG_DFL);\n-\t\traise(SIGPIPE);\n-\t\t/* Should never happen, but just in case... */\n-\t\texit(141);\n-\t}\n-}\n-\n void child_process_init(struct child_process *child)\n {\n \tmemset(child, 0, sizeof(*child));\n@@ -647,7 +634,7 @@ int in_async(void)\n \treturn !pthread_equal(main_thread, pthread_self());\n }\n \n-void NORETURN async_exit(int code)\n+static void NORETURN async_exit(int code)\n {\n \tpthread_exit((void *)(intptr_t)code);\n }\n@@ -697,13 +684,26 @@ int in_async(void)\n \treturn process_is_async;\n }\n \n-void NORETURN async_exit(int code)\n+static void NORETURN async_exit(int code)\n {\n \texit(code);\n }\n \n #endif\n \n+void check_pipe(int err)\n+{\n+\tif (err == EPIPE) {\n+\t\tif (in_async())\n+\t\t\tasync_exit(141);\n+\n+\t\tsignal(SIGPIPE, SIG_DFL);\n+\t\traise(SIGPIPE);\n+\t\t/* Should never happen, but just in case... */\n+\t\texit(141);\n+\t}\n+}\n+\n int start_async(struct async *async)\n {\n \tint need_in, need_out;\ndiff --git a/run-command.h b/run-command.h\nindex e7c5f71..bb89c30 100644\n--- a/run-command.h\n+++ b/run-command.h\n@@ -54,7 +54,6 @@ int finish_command(struct child_process *);\n int finish_command_in_signal(struct child_process *);\n int run_command(struct child_process *);\n \n-void check_pipe(int err);\n \n /*\n * Returns the path to the hook file, or NULL if the hook is missing\n@@ -141,7 +140,7 @@ struct async {\n int start_async(struct async *async);\n int finish_async(struct async *async);\n int in_async(void);\n-void NORETURN async_exit(int code);\n+void check_pipe(int err);\n \n /**\n * This callback should initialize the child process and preload the\n-- \n2.10.0\n"},{"id":"302433","messageId":"58164A5E-AC93-48A9-9139-B69CFB854CA8@gmail.com","threadId":"44135","inReplyTo":"78f2bdd0-f6ad-db5c-f9f2-f90528bc4f77@ramsayjones.plus.com","subject":"Re: [PATCH] run-command: async_exit no longer needs to be public","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-09-23T08:26:02Z","receivedAt":"2016-09-23T08:26:14Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 22 Sep 2016, at 18:56, Ramsay Jones wrote:\n> \n> \n> Signed-off-by: Ramsay Jones \n> ---\n> \n> Hi Lars,\n> \n> If you need to re-roll your 'ls/filter-process' branch, could you please\n> squash this into the relevant commit c42a4cbc (\"run-command: move check_pipe()\n> from write_or_die to run_command\", 20-09-2016).\n> \n> [Note that commit 9658846c (\"write_or_die: handle EPIPE in async threads\",\n> 24-02-2016) introduced async_exit() specifically for use in the implementation\n> of check_pipe(). Now that you have moved check_pipe() into run-command.c,\n> it no longer needs to be public.]\n\nHi Ramsay,\n\nthanks for noticing this. I actually hope that I don't need another re-roll :-)\nIf I don't re-roll. Should I make a patch with this cleanup or do you\ntake care of it?\n\nThanks,\nLars\n\n"},{"id":"302451","messageId":"xmqqmviy1qux.fsf@gitster.mtv.corp.google.com","threadId":"44135","inReplyTo":"58164A5E-AC93-48A9-9139-B69CFB854CA8@gmail.com","subject":"Re: [PATCH] run-command: async_exit no longer needs to be public","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-23T17:13:42Z","receivedAt":"2016-09-23T17:13:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Lars Schneider writes:\n\n>> If you need to re-roll your 'ls/filter-process' branch, could you please\n>> squash this into the relevant commit c42a4cbc (\"run-command: move check_pipe()\n>> from write_or_die to run_command\", 20-09-2016).\n>> \n>> [Note that commit 9658846c (\"write_or_die: handle EPIPE in async threads\",\n>> 24-02-2016) introduced async_exit() specifically for use in the implementation\n>> of check_pipe(). Now that you have moved check_pipe() into run-command.c,\n>> it no longer needs to be public.]\n>\n> Hi Ramsay,\n>\n> thanks for noticing this. I actually hope that I don't need another re-roll :-)\n> If I don't re-roll. Should I make a patch with this cleanup or do you\n> take care of it?\n\nI can just squash the the patch you are responding to into c42a4cbc,\nwith an additional paragraph \"While at it, retire async_exit() as a\npublic function as it no longer is called outside run-command API\nimplementation\", or something like that.\n\nI do not offhand know if the topic is otherwise ready as-is, or\nneeds further work. When you need to reroll, you'd also need to\nfetch from the result of the above from me first and then start your\nwork from it, though, if we go that route.\n\n\n"},{"id":"302455","messageId":"6DE6D03D-F496-4FAB-866A-BC6802312B43@gmail.com","threadId":"44135","inReplyTo":"xmqqmviy1qux.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] run-command: async_exit no longer needs to be public","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-09-23T18:56:34Z","receivedAt":"2016-09-23T18:56:43Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 23 Sep 2016, at 19:13, Junio C Hamano wrote:\n> \n> Lars Schneider writes:\n> \n>>> If you need to re-roll your 'ls/filter-process' branch, could you please\n>>> squash this into the relevant commit c42a4cbc (\"run-command: move check_pipe()\n>>> from write_or_die to run_command\", 20-09-2016).\n>>> \n>>> [Note that commit 9658846c (\"write_or_die: handle EPIPE in async threads\",\n>>> 24-02-2016) introduced async_exit() specifically for use in the implementation\n>>> of check_pipe(). Now that you have moved check_pipe() into run-command.c,\n>>> it no longer needs to be public.]\n>> \n>> Hi Ramsay,\n>> \n>> thanks for noticing this. I actually hope that I don't need another re-roll :-)\n>> If I don't re-roll. Should I make a patch with this cleanup or do you\n>> take care of it?\n> \n> I can just squash the the patch you are responding to into c42a4cbc,\n> with an additional paragraph \"While at it, retire async_exit() as a\n> public function as it no longer is called outside run-command API\n> implementation\", or something like that.\n> \n> I do not offhand know if the topic is otherwise ready as-is, or\n> needs further work. When you need to reroll, you'd also need to\n> fetch from the result of the above from me first and then start your\n> work from it, though, if we go that route.\n\nSounds good to me!\n\nThank you, Junio!"},{"id":"302458","messageId":"xmqq60pm1kp3.fsf@gitster.mtv.corp.google.com","threadId":"44135","inReplyTo":"6DE6D03D-F496-4FAB-866A-BC6802312B43@gmail.com","subject":"Re: [PATCH] run-command: async_exit no longer needs to be public","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-23T19:26:48Z","receivedAt":"2016-09-23T19:26:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Lars Schneider writes:\n\n>> I do not offhand know if the topic is otherwise ready as-is, or\n>> needs further work. When you need to reroll, you'd also need to\n>> fetch from the result of the above from me first and then start your\n>> work from it, though, if we go that route.\n>\n> Sounds good to me!\n\nOK, here is what I queued, then.\n\n-- >8 --\nFrom: Lars Schneider \nDate: Tue, 20 Sep 2016 21:02:39 +0200\nSubject: [PATCH] run-command: move check_pipe() from write_or_die to\n run_command\n\nMove check_pipe() to run_command and make it public. This is necessary\nto call the function from pkt-line in a subsequent patch.\n\nWhile at it, make async_exit() static to run_command.c as it is no\nlonger used from outside.\n\nSigned-off-by: Lars Schneider \nSigned-off-by: Ramsay Jones \nSigned-off-by: Junio C Hamano \n---\n run-command.c | 17 +++++++++++++++--\n run-command.h | 2 +-\n write_or_die.c | 13 -------------\n 3 files changed, 16 insertions(+), 16 deletions(-)\n\ndiff --git a/run-command.c b/run-command.c\nindex 5a4dbb6..3269362 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -634,7 +634,7 @@ int in_async(void)\n \treturn !pthread_equal(main_thread, pthread_self());\n }\n \n-void NORETURN async_exit(int code)\n+static void NORETURN async_exit(int code)\n {\n \tpthread_exit((void *)(intptr_t)code);\n }\n@@ -684,13 +684,26 @@ int in_async(void)\n \treturn process_is_async;\n }\n \n-void NORETURN async_exit(int code)\n+static void NORETURN async_exit(int code)\n {\n \texit(code);\n }\n \n #endif\n \n+void check_pipe(int err)\n+{\n+\tif (err == EPIPE) {\n+\t\tif (in_async())\n+\t\t\tasync_exit(141);\n+\n+\t\tsignal(SIGPIPE, SIG_DFL);\n+\t\traise(SIGPIPE);\n+\t\t/* Should never happen, but just in case... */\n+\t\texit(141);\n+\t}\n+}\n+\n int start_async(struct async *async)\n {\n \tint need_in, need_out;\ndiff --git a/run-command.h b/run-command.h\nindex 5066649..cf29a31 100644\n--- a/run-command.h\n+++ b/run-command.h\n@@ -139,7 +139,7 @@ struct async {\n int start_async(struct async *async);\n int finish_async(struct async *async);\n int in_async(void);\n-void NORETURN async_exit(int code);\n+void check_pipe(int err);\n \n /**\n * This callback should initialize the child process and preload the\ndiff --git a/write_or_die.c b/write_or_die.c\nindex 0734432..eab8c8d 100644\n--- a/write_or_die.c\n+++ b/write_or_die.c\n@@ -1,19 +1,6 @@\n #include \"cache.h\"\n #include \"run-command.h\"\n \n-static void check_pipe(int err)\n-{\n-\tif (err == EPIPE) {\n-\t\tif (in_async())\n-\t\t\tasync_exit(141);\n-\n-\t\tsignal(SIGPIPE, SIG_DFL);\n-\t\traise(SIGPIPE);\n-\t\t/* Should never happen, but just in case... */\n-\t\texit(141);\n-\t}\n-}\n-\n /*\n * Some cases use stdio, but want to flush after the write\n * to get error handling (and to get better interactive\n-- \n2.10.0-530-g67247c9\n\n"},{"id":"302476","messageId":"866eac55-92cb-d034-3ba8-faa84d8c8a03@ramsayjones.plus.com","threadId":"44135","inReplyTo":"xmqq60pm1kp3.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] run-command: async_exit no longer needs to be public","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2016-09-24T01:04:47Z","receivedAt":"2016-09-24T01:04:58Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 23/09/16 20:26, Junio C Hamano wrote:\n> Lars Schneider writes:\n> \n>>> I do not offhand know if the topic is otherwise ready as-is, or\n>>> needs further work. When you need to reroll, you'd also need to\n>>> fetch from the result of the above from me first and then start your\n>>> work from it, though, if we go that route.\n>>\n>> Sounds good to me!\n> \n> OK, here is what I queued, then.\n\nThis looks good to me. Thanks!\n\nATB,\nRamsay Jones\n\n\n"}]}