{"thread":{"id":"40846","subject":"[PATCHv2] run-command: detect finished children by closed pipe rather than waitpid","startedAt":"2015-11-20T21:08:39Z","lastAt":"2015-11-23T19:01:12Z","messageCount":6,"participants":["Stefan Beller","Jonathan Nieder","Johannes Sixt","Torsten Bögershausen"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"273560","messageId":"1448053719-28340-1-git-send-email-sbeller@google.com","threadId":"40846","inReplyTo":null,"subject":"[PATCHv2] run-command: detect finished children by closed pipe rather than waitpid","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-11-20T21:08:39Z","receivedAt":"2015-11-20T21:08:39Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Detect if a child stopped working by checking if their stderr pipe\nwas closed instead of checking their state with waitpid.\nAs waitpid is not fully working in Windows, this is an approach which\nallows for better cross platform operation. (It's less code, too)\n\nPreviously we did not close the read pipe of finished children, which we\ndo now.\n\nThe old way missed some messages on an early abort. We just killed the\nchildren and did not bother to look what was left over. With this approach\nwe'd send a signal to the children and wait for them to close the pipe to\nhave all the messages (including possible \"killed by signal 15\" messages).\n\nTo have the test suite passing as before, we allow for real graceful\nabortion now. In case the user wishes to abort parallel execution\nthe user needs to provide either the signal used to kill all children\nor the children are let run until they finish normally.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n\nThis applies on top of peff/sb/submodule-parallel-fetch.\nIt is a resend without modification from Nov. 11th.\n\n run-command.c      | 141 +++++++++++++++++++++++------------------------------\n run-command.h      |  12 +++--\n submodule.c        |   3 --\n test-run-command.c |   3 --\n 4 files changed, 69 insertions(+), 90 deletions(-)\n\ndiff --git a/run-command.c b/run-command.c\nindex 07424e9..db4d916 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -858,6 +858,12 @@ int capture_command(struct child_process *cmd, struct strbuf *buf, size_t hint)\n \treturn finish_command(cmd);\n }\n \n+enum child_state {\n+\tGIT_CP_FREE,\n+\tGIT_CP_WORKING,\n+\tGIT_CP_WAIT_CLEANUP,\n+};\n+\n static struct parallel_processes {\n \tvoid *data;\n \n@@ -869,7 +875,7 @@ static struct parallel_processes {\n \ttask_finished_fn task_finished;\n \n \tstruct {\n-\t\tunsigned in_use : 1;\n+\t\tenum child_state state;\n \t\tstruct child_process process;\n \t\tstruct strbuf err;\n \t\tvoid *data;\n@@ -923,7 +929,7 @@ static void kill_children(struct parallel_processes *pp, int signo)\n \tint i, n = pp->max_processes;\n \n \tfor (i = 0; i < n; i++)\n-\t\tif (pp->children[i].in_use)\n+\t\tif (pp->children[i].state == GIT_CP_WORKING)\n \t\t\tkill(pp->children[i].process.pid, signo);\n }\n \n@@ -967,7 +973,7 @@ static struct parallel_processes *pp_init(int n,\n \tfor (i = 0; i < n; i++) {\n \t\tstrbuf_init(&pp->children[i].err, 0);\n \t\tchild_process_init(&pp->children[i].process);\n-\t\tpp->pfd[i].events = POLLIN;\n+\t\tpp->pfd[i].events = POLLIN | POLLHUP;\n \t\tpp->pfd[i].fd = -1;\n \t}\n \tsigchain_push_common(handle_children_on_signal);\n@@ -1000,39 +1006,46 @@ static void pp_cleanup(struct parallel_processes *pp)\n  *  0 if a new task was started.\n  *  1 if no new jobs was started (get_next_task ran out of work, non critical\n  *    problem with starting a new command)\n- * -1 no new job was started, user wishes to shutdown early.\n+ * <0 no new job was started, user wishes to shutdown early. Use negative code\n+ *    to signal the children.\n  */\n static int pp_start_one(struct parallel_processes *pp)\n {\n-\tint i;\n+\tint i, code;\n \n \tfor (i = 0; i < pp->max_processes; i++)\n-\t\tif (!pp->children[i].in_use)\n+\t\tif (pp->children[i].state == GIT_CP_FREE)\n \t\t\tbreak;\n \tif (i == pp->max_processes)\n \t\tdie(\"BUG: bookkeeping is hard\");\n \n-\tif (!pp->get_next_task(&pp->children[i].data,\n-\t\t\t       &pp->children[i].process,\n-\t\t\t       &pp->children[i].err,\n-\t\t\t       pp->data)) {\n+\tcode = pp->get_next_task(&pp->children[i].data,\n+\t\t\t\t &pp->children[i].process,\n+\t\t\t\t &pp->children[i].err,\n+\t\t\t\t pp->data);\n+\tif (!code) {\n \t\tstrbuf_addbuf(&pp->buffered_output, &pp->children[i].err);\n \t\tstrbuf_reset(&pp->children[i].err);\n \t\treturn 1;\n \t}\n+\tpp->children[i].process.err = -1;\n+\tpp->children[i].process.stdout_to_stderr = 1;\n+\tpp->children[i].process.no_stdin = 1;\n \n \tif (start_command(&pp->children[i].process)) {\n-\t\tint code = pp->start_failure(&pp->children[i].process,\n-\t\t\t\t\t     &pp->children[i].err,\n-\t\t\t\t\t     pp->data,\n-\t\t\t\t\t     &pp->children[i].data);\n+\t\tcode = pp->start_failure(&pp->children[i].process,\n+\t\t\t\t\t &pp->children[i].err,\n+\t\t\t\t\t pp->data,\n+\t\t\t\t\t &pp->children[i].data);\n \t\tstrbuf_addbuf(&pp->buffered_output, &pp->children[i].err);\n \t\tstrbuf_reset(&pp->children[i].err);\n-\t\treturn code ? -1 : 1;\n+\t\tif (code)\n+\t\t\tpp->shutdown = 1;\n+\t\treturn code;\n \t}\n \n \tpp->nr_processes++;\n-\tpp->children[i].in_use = 1;\n+\tpp->children[i].state = GIT_CP_WORKING;\n \tpp->pfd[i].fd = pp->children[i].process.err;\n \treturn 0;\n }\n@@ -1050,19 +1063,24 @@ static void pp_buffer_stderr(struct parallel_processes *pp, int output_timeout)\n \n \t/* Buffer output from all pipes. */\n \tfor (i = 0; i < pp->max_processes; i++) {\n-\t\tif (pp->children[i].in_use &&\n-\t\t    pp->pfd[i].revents & POLLIN)\n-\t\t\tif (strbuf_read_once(&pp->children[i].err,\n-\t\t\t\t\t     pp->children[i].process.err, 0) < 0)\n+\t\tif (pp->children[i].state == GIT_CP_WORKING &&\n+\t\t    pp->pfd[i].revents & (POLLIN | POLLHUP)) {\n+\t\t\tint n = strbuf_read_once(&pp->children[i].err,\n+\t\t\t\t\t\t pp->children[i].process.err, 0);\n+\t\t\tif (n == 0) {\n+\t\t\t\tclose(pp->children[i].process.err);\n+\t\t\t\tpp->children[i].state = GIT_CP_WAIT_CLEANUP;\n+\t\t\t} else if (n < 0)\n \t\t\t\tif (errno != EAGAIN)\n \t\t\t\t\tdie_errno(\"read\");\n+\t\t}\n \t}\n }\n \n static void pp_output(struct parallel_processes *pp)\n {\n \tint i = pp->output_owner;\n-\tif (pp->children[i].in_use &&\n+\tif (pp->children[i].state == GIT_CP_WORKING &&\n \t    pp->children[i].err.len) {\n \t\tfputs(pp->children[i].err.buf, stderr);\n \t\tstrbuf_reset(&pp->children[i].err);\n@@ -1071,70 +1089,31 @@ static void pp_output(struct parallel_processes *pp)\n \n static int pp_collect_finished(struct parallel_processes *pp)\n {\n-\tint i = 0;\n-\tpid_t pid;\n-\tint wait_status, code;\n+\tint i, code;\n \tint n = pp->max_processes;\n \tint result = 0;\n \n \twhile (pp->nr_processes > 0) {\n-\t\tpid = waitpid(-1, &wait_status, WNOHANG);\n-\t\tif (pid == 0)\n-\t\t\tbreak;\n-\n-\t\tif (pid < 0)\n-\t\t\tdie_errno(\"wait\");\n-\n \t\tfor (i = 0; i < pp->max_processes; i++)\n-\t\t\tif (pp->children[i].in_use &&\n-\t\t\t    pid == pp->children[i].process.pid)\n+\t\t\tif (pp->children[i].state == GIT_CP_WAIT_CLEANUP)\n \t\t\t\tbreak;\n \t\tif (i == pp->max_processes)\n-\t\t\tdie(\"BUG: found a child process we were not aware of\");\n-\n-\t\tif (strbuf_read(&pp->children[i].err,\n-\t\t\t\tpp->children[i].process.err, 0) < 0)\n-\t\t\tdie_errno(\"strbuf_read\");\n-\n-\t\tif (WIFSIGNALED(wait_status)) {\n-\t\t\tcode = WTERMSIG(wait_status);\n-\t\t\tif (!pp->shutdown &&\n-\t\t\t    code != SIGINT && code != SIGQUIT)\n-\t\t\t\tstrbuf_addf(&pp->children[i].err,\n-\t\t\t\t\t    \"%s died of signal %d\",\n-\t\t\t\t\t    pp->children[i].process.argv[0],\n-\t\t\t\t\t    code);\n-\t\t\t/*\n-\t\t\t * This return value is chosen so that code & 0xff\n-\t\t\t * mimics the exit code that a POSIX shell would report for\n-\t\t\t * a program that died from this signal.\n-\t\t\t */\n-\t\t\tcode += 128;\n-\t\t} else if (WIFEXITED(wait_status)) {\n-\t\t\tcode = WEXITSTATUS(wait_status);\n-\t\t\t/*\n-\t\t\t * Convert special exit code when execvp failed.\n-\t\t\t */\n-\t\t\tif (code == 127) {\n-\t\t\t\tcode = -1;\n-\t\t\t\terrno = ENOENT;\n-\t\t\t}\n-\t\t} else {\n-\t\t\tstrbuf_addf(&pp->children[i].err,\n-\t\t\t\t    \"waitpid is confused (%s)\",\n-\t\t\t\t    pp->children[i].process.argv[0]);\n-\t\t\tcode = -1;\n-\t\t}\n+\t\t\tbreak;\n+\n+\t\tcode = finish_command(&pp->children[i].process);\n+\n+\t\tcode = pp->task_finished(code, &pp->children[i].process,\n+\t\t\t\t\t &pp->children[i].err, pp->data,\n+\t\t\t\t\t &pp->children[i].data);\n \n-\t\tif (pp->task_finished(code, &pp->children[i].process,\n-\t\t\t\t      &pp->children[i].err, pp->data,\n-\t\t\t\t      &pp->children[i].data))\n-\t\t\tresult = 1;\n+\t\tif (code)\n+\t\t\tresult = code;\n+\t\tif (code < 0)\n+\t\t\tbreak;\n \n \t\tpp->nr_processes--;\n-\t\tpp->children[i].in_use = 0;\n+\t\tpp->children[i].state = GIT_CP_FREE;\n \t\tpp->pfd[i].fd = -1;\n-\t\tchild_process_deinit(&pp->children[i].process);\n \t\tchild_process_init(&pp->children[i].process);\n \n \t\tif (i != pp->output_owner) {\n@@ -1157,7 +1136,7 @@ static int pp_collect_finished(struct parallel_processes *pp)\n \t\t\t * running process time.\n \t\t\t */\n \t\t\tfor (i = 0; i < n; i++)\n-\t\t\t\tif (pp->children[(pp->output_owner + i) % n].in_use)\n+\t\t\t\tif (pp->children[(pp->output_owner + i) % n].state == GIT_CP_WORKING)\n \t\t\t\t\tbreak;\n \t\t\tpp->output_owner = (pp->output_owner + i) % n;\n \t\t}\n@@ -1171,7 +1150,7 @@ int run_processes_parallel(int n,\n \t\t\t   task_finished_fn task_finished,\n \t\t\t   void *pp_cb)\n {\n-\tint i;\n+\tint i, code;\n \tint output_timeout = 100;\n \tint spawn_cap = 4;\n \tstruct parallel_processes *pp;\n@@ -1182,12 +1161,12 @@ int run_processes_parallel(int n,\n \t\t    i < spawn_cap && !pp->shutdown &&\n \t\t    pp->nr_processes < pp->max_processes;\n \t\t    i++) {\n-\t\t\tint code = pp_start_one(pp);\n+\t\t\tcode = pp_start_one(pp);\n \t\t\tif (!code)\n \t\t\t\tcontinue;\n \t\t\tif (code < 0) {\n \t\t\t\tpp->shutdown = 1;\n-\t\t\t\tkill_children(pp, SIGTERM);\n+\t\t\t\tkill_children(pp, -code);\n \t\t\t}\n \t\t\tbreak;\n \t\t}\n@@ -1195,9 +1174,11 @@ int run_processes_parallel(int n,\n \t\t\tbreak;\n \t\tpp_buffer_stderr(pp, output_timeout);\n \t\tpp_output(pp);\n-\t\tif (pp_collect_finished(pp)) {\n-\t\t\tkill_children(pp, SIGTERM);\n+\t\tcode = pp_collect_finished(pp);\n+\t\tif (code) {\n \t\t\tpp->shutdown = 1;\n+\t\t\tif (code < 0)\n+\t\t\t\tkill_children(pp, -code);\n \t\t}\n \t}\n \ndiff --git a/run-command.h b/run-command.h\nindex c24aa54..414cc81 100644\n--- a/run-command.h\n+++ b/run-command.h\n@@ -134,6 +134,8 @@ int finish_async(struct async *async);\n  *\n  * Return 1 if the next child is ready to run.\n  * Return 0 if there are currently no more tasks to be processed.\n+ * To send a signal to other child processes for abortion,\n+ * return negative signal code.\n  */\n typedef int (*get_next_task_fn)(void **pp_task_cb,\n \t\t\t\tstruct child_process *cp,\n@@ -151,8 +153,9 @@ typedef int (*get_next_task_fn)(void **pp_task_cb,\n  * pp_cb is the callback cookie as passed into run_processes_parallel,\n  * pp_task_cb is the callback cookie as passed into get_next_task_fn.\n  *\n- * Return 0 to continue the parallel processing. To abort gracefully,\n- * return non zero.\n+ * Return 0 to continue the parallel processing. To abort return non zero.\n+ * To send a signal to other child processes for abortion, return\n+ * negative signal code.\n  */\n typedef int (*start_failure_fn)(struct child_process *cp,\n \t\t\t\tstruct strbuf *err,\n@@ -169,8 +172,9 @@ typedef int (*start_failure_fn)(struct child_process *cp,\n  * pp_cb is the callback cookie as passed into run_processes_parallel,\n  * pp_task_cb is the callback cookie as passed into get_next_task_fn.\n  *\n- * Return 0 to continue the parallel processing. To abort gracefully,\n- * return non zero.\n+ * Return 0 to continue the parallel processing.  To abort return non zero.\n+ * To send a signal to other child processes for abortion, return\n+ * negative signal code.\n  */\n typedef int (*task_finished_fn)(int result,\n \t\t\t\tstruct child_process *cp,\ndiff --git a/submodule.c b/submodule.c\nindex c21b265..281bccd 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -689,9 +689,6 @@ static int get_next_submodule(void **task_cb, struct child_process *cp,\n \t\t\tcp->dir = strbuf_detach(&submodule_path, NULL);\n \t\t\tcp->env = local_repo_env;\n \t\t\tcp->git_cmd = 1;\n-\t\t\tcp->no_stdin = 1;\n-\t\t\tcp->stdout_to_stderr = 1;\n-\t\t\tcp->err = -1;\n \t\t\tif (!spf->quiet)\n \t\t\t\tstrbuf_addf(err, \"Fetching submodule %s%s\\n\",\n \t\t\t\t\t    spf->prefix, ce->name);\ndiff --git a/test-run-command.c b/test-run-command.c\nindex 13e5d44..b1f04d1 100644\n--- a/test-run-command.c\n+++ b/test-run-command.c\n@@ -26,9 +26,6 @@ static int parallel_next(void** task_cb,\n \t\treturn 0;\n \n \targv_array_pushv(&cp->args, d->argv);\n-\tcp->stdout_to_stderr = 1;\n-\tcp->no_stdin = 1;\n-\tcp->err = -1;\n \tstrbuf_addf(err, \"preloaded output of a child\\n\");\n \tnumber_callbacks++;\n \treturn 1;\n-- \n2.6.1.256.g2277835.dirty\n"},{"id":"273562","messageId":"20151120220544.GC18255@google.com","threadId":"40846","inReplyTo":"1448053719-28340-1-git-send-email-sbeller@google.com","subject":"Re: [PATCHv2] run-command: detect finished children by closed pipe rather than waitpid","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2015-11-20T22:05:44Z","receivedAt":"2015-11-20T22:05:44Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Stefan Beller wrote:\n\n> Detect if a child stopped working by checking if their stderr pipe\n> was closed instead of checking their state with waitpid.\n> As waitpid is not fully working in Windows, this is an approach which\n> allows for better cross platform operation. (It's less code, too)\n\nCan you say more about what is broken about waitpid on Windows?\n\nI ask because it's possible for a child to close stderr without\nintending to be finished.  That might be okay here (though the commit\nsubject doesn't explain so, it is only affecting the workqueue\ninterface that runs commands in parallel and not the normal\nrun-command interface) but would need some documentation and could be\ncounterintuitive.\n\nSo if there's a simple way to get waitpid to work, that seems\npreferrable.\n\nThanks,\nJonathan\n"},{"id":"273563","messageId":"564FABA3.2040100@kdbg.org","threadId":"40846","inReplyTo":"20151120220544.GC18255@google.com","subject":"Re: [PATCHv2] run-command: detect finished children by closed pipe rather than waitpid","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2015-11-20T23:24:19Z","receivedAt":"2015-11-20T23:24:19Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 20.11.2015 um 23:05 schrieb Jonathan Nieder:\n> Stefan Beller wrote:\n>\n>> Detect if a child stopped working by checking if their stderr pipe\n>> was closed instead of checking their state with waitpid.\n>> As waitpid is not fully working in Windows, this is an approach which\n>> allows for better cross platform operation. (It's less code, too)\n>\n> Can you say more about what is broken about waitpid on Windows?\n\nwaitpid(-1, ...) is not implemented on Windows.\n\nIs it necessary to mention waitpid here at all? The most compelling \nreason to write the infrastructure in this new way is that it is much \nmore in line with the usual \"start_command, read-until-EOF, \nfinish_command\" sequence.\n\n> I ask because it's possible for a child to close stderr without\n> intending to be finished.  That might be okay here (though the commit\n> subject doesn't explain so, it is only affecting the workqueue\n> interface that runs commands in parallel and not the normal\n> run-command interface) but would need some documentation and could be\n> counterintuitive.\n\nIt could be spelled out more clearly. The children have both their stdin \nand stdout redirected to the same writable end. On the parent side, we \nhave to deal only with \"stderr\" simply because our child_process \nfacility has everything to redirect like \">&2\" (the stdout_to_stderr \nflag), but nothing for \"2>&1\".\n\nYeah, it's still possible that the child closes both stdout and stderr \nlong before it dies, but that would really be far, far outside the norm.\n\n> So if there's a simple way to get waitpid to work, that seems\n> preferrable.\n\nIt would be possible, but not simple, to make waitpid on Windows \nsuitable for the original code, but that does not make the original code \npreferrable.\n\n-- Hannes\n"},{"id":"273568","messageId":"5650161E.9060305@web.de","threadId":"40846","inReplyTo":"1448053719-28340-1-git-send-email-sbeller@google.com","subject":"Re: [PATCHv2] run-command: detect finished children by closed pipe rather than waitpid","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2015-11-21T06:58:38Z","receivedAt":"2015-11-21T06:58:38Z","isPatch":false,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 2015-11-20 22.08, Stefan Beller wrote:\nThe patch looks good at first glance, one minor remark below:\n> \n> diff --git a/run-command.c b/run-command.c\n\n> @@ -1071,70 +1089,31 @@ static void pp_output(struct parallel_processes *pp)\n>  \n>  static int pp_collect_finished(struct parallel_processes *pp)\n>  {\n> -\tint i = 0;\n> -\tpid_t pid;\n> -\tint wait_status, code;\n> +\tint i, code;\n\ncode is probably \"return code\"?\nwoud \"ret_value\", \"res\" or \"rc\" make that more clear ?\n"},{"id":"273612","messageId":"CAGZ79kYs9+UTx__yccy6qZY+Xn4onWx3zpH51pzHPn8Hz_FMww@mail.gmail.com","threadId":"40846","inReplyTo":"5650161E.9060305@web.de","subject":"Re: [PATCHv2] run-command: detect finished children by closed pipe rather than waitpid","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-11-23T18:44:51Z","receivedAt":"2015-11-23T18:44:51Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Fri, Nov 20, 2015 at 10:58 PM, Torsten Bögershausen <tboegi@web.de> wrote:\n> On 2015-11-20 22.08, Stefan Beller wrote:\n> The patch looks good at first glance, one minor remark below:\n>>\n>> diff --git a/run-command.c b/run-command.c\n>\n>> @@ -1071,70 +1089,31 @@ static void pp_output(struct parallel_processes *pp)\n>>\n>>  static int pp_collect_finished(struct parallel_processes *pp)\n>>  {\n>> -     int i = 0;\n>> -     pid_t pid;\n>> -     int wait_status, code;\n>> +     int i, code;\n>\n> code is probably \"return code\"?\n> woud \"ret_value\", \"res\" or \"rc\" make that more clear ?\n>\n\nThe return value of invoked process, yes.\n\"ret_value\", \"res\" sound too much like our own future return value\n(by convention of naming its own return value ret_value or similar).\n\nInstead of return code, I may settle with `exit_status`?\n"},{"id":"273618","messageId":"CAGZ79kYte2-1ddbFOnWL5NFAuK6auLDD2n47Dz+6FcZCpC1vTA@mail.gmail.com","threadId":"40846","inReplyTo":"CAGZ79kYs9+UTx__yccy6qZY+Xn4onWx3zpH51pzHPn8Hz_FMww@mail.gmail.com","subject":"Re: [PATCHv2] run-command: detect finished children by closed pipe rather than waitpid","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-11-23T19:01:12Z","receivedAt":"2015-11-23T19:01:12Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Mon, Nov 23, 2015 at 10:44 AM, Stefan Beller <sbeller@google.com> wrote:\n> On Fri, Nov 20, 2015 at 10:58 PM, Torsten Bögershausen <tboegi@web.de> wrote:\n>> On 2015-11-20 22.08, Stefan Beller wrote:\n>> The patch looks good at first glance, one minor remark below:\n>>>\n>>> diff --git a/run-command.c b/run-command.c\n>>\n>>> @@ -1071,70 +1089,31 @@ static void pp_output(struct parallel_processes *pp)\n>>>\n>>>  static int pp_collect_finished(struct parallel_processes *pp)\n>>>  {\n>>> -     int i = 0;\n>>> -     pid_t pid;\n>>> -     int wait_status, code;\n>>> +     int i, code;\n>>\n>> code is probably \"return code\"?\n>> woud \"ret_value\", \"res\" or \"rc\" make that more clear ?\n>>\n>\n\nAlthough looking through the code, we have lots of functions\nhaving a local `code` variable, so we may want to preserve consistency\nacross the different functions to have a `code`which contains the return\nvalue of the process or function invoked.\n\nWe had the `code` already in pp_collect_finished, so I'd like to not\nrename a variable (which was used for the same purpose) in this patch.\n\nIn pp_start_one we introduce a new variable `code` which contains\nthe return from the user callback function, so I would understand if\nwe were arguing there.\n\nThat said, I plan to resend with a reworded commit message later today.\n"}]}