| Message ID | 20181112144654.GA7400@sigill.intra.peff.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | caching loose objects | expand |
On 11/12/2018 9:46 AM, Jeff King wrote: > The run-command API makes no promises about what is left in a struct > child_process after a command finishes, and it's not safe to simply > reuse it again for a similar command. In particular: > > - if you use child->args or child->env_array, they are cleared after > finish_command() > > - likewise, start_command() may point child->argv at child->args->argv; > reusing that would lead to accessing freed memory > > - the in/out/err may hold pipe descriptors from the previous run Thanks! This is helpful information. > These two calls are _probably_ OK because they do not use any of those > features. But it's only by chance, and may break in the future; let's > reinitialize our struct for each program we run. > > Signed-off-by: Jeff King <peff@peff.net> > --- > builtin/fsck.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/builtin/fsck.c b/builtin/fsck.c > index 06eb421720..b10f2b154c 100644 > --- a/builtin/fsck.c > +++ b/builtin/fsck.c > @@ -841,6 +841,9 @@ int cmd_fsck(int argc, const char **argv, const char *prefix) > > prepare_alt_odb(the_repository); > for (alt = the_repository->objects->alt_odb_list; alt; alt = alt->next) { > + child_process_init(&commit_graph_verify); > + commit_graph_verify.argv = verify_argv; > + commit_graph_verify.git_cmd = 1; > verify_argv[2] = "--object-dir"; > verify_argv[3] = alt->path; > if (run_command(&commit_graph_verify)) > @@ -859,6 +862,9 @@ int cmd_fsck(int argc, const char **argv, const char *prefix) > > prepare_alt_odb(the_repository); > for (alt = the_repository->objects->alt_odb_list; alt; alt = alt->next) { > + child_process_init(&midx_verify); > + midx_verify.argv = midx_argv; > + midx_verify.git_cmd = 1; > midx_argv[2] = "--object-dir"; > midx_argv[3] = alt->path; > if (run_command(&midx_verify)) Looks good to me. -Stolee
diff --git a/builtin/fsck.c b/builtin/fsck.c index 06eb421720..b10f2b154c 100644 --- a/builtin/fsck.c +++ b/builtin/fsck.c @@ -841,6 +841,9 @@ int cmd_fsck(int argc, const char **argv, const char *prefix) prepare_alt_odb(the_repository); for (alt = the_repository->objects->alt_odb_list; alt; alt = alt->next) { + child_process_init(&commit_graph_verify); + commit_graph_verify.argv = verify_argv; + commit_graph_verify.git_cmd = 1; verify_argv[2] = "--object-dir"; verify_argv[3] = alt->path; if (run_command(&commit_graph_verify)) @@ -859,6 +862,9 @@ int cmd_fsck(int argc, const char **argv, const char *prefix) prepare_alt_odb(the_repository); for (alt = the_repository->objects->alt_odb_list; alt; alt = alt->next) { + child_process_init(&midx_verify); + midx_verify.argv = midx_argv; + midx_verify.git_cmd = 1; midx_argv[2] = "--object-dir"; midx_argv[3] = alt->path; if (run_command(&midx_verify))
The run-command API makes no promises about what is left in a struct child_process after a command finishes, and it's not safe to simply reuse it again for a similar command. In particular: - if you use child->args or child->env_array, they are cleared after finish_command() - likewise, start_command() may point child->argv at child->args->argv; reusing that would lead to accessing freed memory - the in/out/err may hold pipe descriptors from the previous run These two calls are _probably_ OK because they do not use any of those features. But it's only by chance, and may break in the future; let's reinitialize our struct for each program we run. Signed-off-by: Jeff King <peff@peff.net> --- builtin/fsck.c | 6 ++++++ 1 file changed, 6 insertions(+)