Brandon Williams <bmw...@google.com> writes: > On 05/01, Stefan Beller wrote: >> While fixing the leak of `cp`, reuse it instead of having to declare >> another struct child_process. >> >> Signed-off-by: Stefan Beller <sbel...@google.com> > > This shouldn't be needed as 'finish_command' does the cleanup for you by > calling 'child_prcoess_clear()'.
Yes, indeed. It might not hurt to update the code so that the cp is reused in the second run instead of using a separate instance cp1, but that is a topic separate from fixing a non-existing leak. >> --- >> submodule.c | 15 +++++++++------ >> 1 file changed, 9 insertions(+), 6 deletions(-) >> >> diff --git a/submodule.c b/submodule.c >> index d3299e29c0..cd098cf12b 100644 >> --- a/submodule.c >> +++ b/submodule.c >> @@ -1466,17 +1466,19 @@ int submodule_move_head(const char *path, >> goto out; >> } >> >> + child_process_clear(&cp); >> + >> if (!(flags & SUBMODULE_MOVE_HEAD_DRY_RUN)) { >> if (new) { >> - struct child_process cp1 = CHILD_PROCESS_INIT; >> + child_process_init(&cp); >> /* also set the HEAD accordingly */ >> - cp1.git_cmd = 1; >> - cp1.no_stdin = 1; >> - cp1.dir = path; >> + cp.git_cmd = 1; >> + cp.no_stdin = 1; >> + cp.dir = path; >> >> - argv_array_pushl(&cp1.args, "update-ref", "HEAD", new, >> NULL); >> + argv_array_pushl(&cp.args, "update-ref", "HEAD", new, >> NULL); >> >> - if (run_command(&cp1)) { >> + if (run_command(&cp)) { >> ret = -1; >> goto out; >> } >> @@ -1492,6 +1494,7 @@ int submodule_move_head(const char *path, >> } >> } >> out: >> + child_process_clear(&cp); >> return ret; >> } >> >> -- >> 2.13.0.rc1.1.gbc33f0f778 >>