run-command: do not pass child process data into callbacks
The expected way to pass data into the callback is to pass them via the customizable callback pointer. The error reporting in default_{start_failure, task_finished} is not user friendly enough, that we want to encourage using the child data for such purposes. Furthermore the struct child data is cleaned by the run-command API, before we access them in the callbacks, leading to use-after-free situations. Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Stefan Beller committed
Feb 29, 2016 at 13:57 UTC
2a73b3dad09ef162eb5917e9e0d01d7c306f6b35
4 files changed
+9
-32
run-command.c
+3
-21
@@ -902,35 +902,18 @@ struct parallel_processes {
902
struct strbuf buffered_output; /* of finished children */
903
};
904
905
-static int default_start_failure(struct child_process *cp,
906
- struct strbuf *err,
905
+static int default_start_failure(struct strbuf *err,
906
void *pp_cb,
907
void *pp_task_cb)
908
{
910
- int i;
911
-
912
- strbuf_addstr(err, "Starting a child failed:");
913
- for (i = 0; cp->argv[i]; i++)
914
- strbuf_addf(err, " %s", cp->argv[i]);
915
-
909
return 0;
910
}
911
912
static int default_task_finished(int result,
920
- struct child_process *cp,
913
struct strbuf *err,
914
void *pp_cb,
915
void *pp_task_cb)
916
{
925
- int i;
926
-
927
- if (!result)
928
- return 0;
929
-
930
- strbuf_addf(err, "A child failed with return code %d:", result);
931
- for (i = 0; cp->argv[i]; i++)
932
- strbuf_addf(err, " %s", cp->argv[i]);
933
-
917
return 0;
918
}
919
@@ -1048,8 +1031,7 @@ static int pp_start_one(struct parallel_processes *pp)
1031
pp->children[i].process.no_stdin = 1;
1032
1033
if (start_command(&pp->children[i].process)) {
1051
- code = pp->start_failure(&pp->children[i].process,
1052
- &pp->children[i].err,
1034
+ code = pp->start_failure(&pp->children[i].err,
1035
pp->data,
1036
&pp->children[i].data);
1037
strbuf_addbuf(&pp->buffered_output, &pp->children[i].err);
@@ -1117,7 +1099,7 @@ static int pp_collect_finished(struct parallel_processes *pp)
1099
1100
code = finish_command(&pp->children[i].process);
1101
1120
- code = pp->task_finished(code, &pp->children[i].process,
1102
+ code = pp->task_finished(code,
1103
&pp->children[i].err, pp->data,
1104
&pp->children[i].data);
1105
run-command.h
+3
-6
@@ -158,8 +158,7 @@ typedef int (*get_next_task_fn)(struct child_process *cp,
158
* To send a signal to other child processes for abortion, return
159
* the negative signal number.
160
*/
161
-typedef int (*start_failure_fn)(struct child_process *cp,
162
- struct strbuf *err,
161
+typedef int (*start_failure_fn)(struct strbuf *err,
162
void *pp_cb,
163
void *pp_task_cb);
164
@@ -178,7 +177,6 @@ typedef int (*start_failure_fn)(struct child_process *cp,
177
* the negative signal number.
178
*/
179
typedef int (*task_finished_fn)(int result,
181
- struct child_process *cp,
180
struct strbuf *err,
181
void *pp_cb,
182
void *pp_task_cb);
@@ -192,9 +190,8 @@ typedef int (*task_finished_fn)(int result,
190
* (both stdout and stderr) is routed to stderr in a manner that output
191
* from different tasks does not interleave.
192
*
195
- * If start_failure_fn or task_finished_fn are NULL, default handlers
196
- * will be used. The default handlers will print an error message on
197
- * error without issuing an emergency stop.
193
+ * start_failure_fn and task_finished_fn can be NULL to omit any
194
+ * special handling.
195
*/
196
int run_processes_parallel(int n,
197
get_next_task_fn,
submodule.c
+3
-4
@@ -705,8 +705,7 @@ static int get_next_submodule(struct child_process *cp,
705
return 0;
706
}
707
708
-static int fetch_start_failure(struct child_process *cp,
709
- struct strbuf *err,
708
+static int fetch_start_failure(struct strbuf *err,
709
void *cb, void *task_cb)
710
{
711
struct submodule_parallel_fetch *spf = cb;
@@ -716,8 +715,8 @@ static int fetch_start_failure(struct child_process *cp,
715
return 0;
716
}
717
719
-static int fetch_finish(int retvalue, struct child_process *cp,
720
- struct strbuf *err, void *cb, void *task_cb)
718
+static int fetch_finish(int retvalue, struct strbuf *err,
719
+ void *cb, void *task_cb)
720
{
721
struct submodule_parallel_fetch *spf = cb;
722
test-run-command.c
-1
@@ -41,7 +41,6 @@ static int no_job(struct child_process *cp,
41
}
42
43
static int task_finished(int result,
44
- struct child_process *cp,
44
struct strbuf *err,
45
void *pp_cb,
46
void *pp_task_cb)