Skip to content

Commit 647e870

Browse files
peffgitster
authored andcommitted
rebase: use child_process_clear() to clean
In the run_am() function, we set up a child_process struct to run "git-am", allocating memory for its args and env strvecs. These are normally cleaned up when we call run_command(). But if we encounter certain errors, we exit the function early and try to clean up ourselves by clearing the am.args field. This leaks the "env" strvec. We should use child_process_clear() instead, which covers both. And more importantly, it future proofs us against the struct ever growing more allocated fields. These are unlikely errors to happen in practice, so they don't actually trigger the leak sanitizer in the tests. But we can add a new test which does exercise one of the paths (and fails SANITIZE=leak without this patch). Signed-off-by: Jeff King <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 0d1bd1d commit 647e870

File tree

2 files changed

+12
-3
lines changed

2 files changed

+12
-3
lines changed

builtin/rebase.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -653,7 +653,7 @@ static int run_am(struct rebase_options *opts)
653653
status = error_errno(_("could not open '%s' for writing"),
654654
rebased_patches);
655655
free(rebased_patches);
656-
strvec_clear(&am.args);
656+
child_process_clear(&am);
657657
return status;
658658
}
659659

@@ -676,7 +676,7 @@ static int run_am(struct rebase_options *opts)
676676
struct reset_head_opts ropts = { 0 };
677677
unlink(rebased_patches);
678678
free(rebased_patches);
679-
strvec_clear(&am.args);
679+
child_process_clear(&am);
680680

681681
ropts.oid = &opts->orig_head->object.oid;
682682
ropts.branch = opts->head_name;
@@ -699,7 +699,7 @@ static int run_am(struct rebase_options *opts)
699699
status = error_errno(_("could not open '%s' for reading"),
700700
rebased_patches);
701701
free(rebased_patches);
702-
strvec_clear(&am.args);
702+
child_process_clear(&am);
703703
return status;
704704
}
705705

t/t3438-rebase-broken-files.sh

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,4 +58,13 @@ test_expect_success 'unknown key in author-script' '
5858
check_resolve_fails
5959
'
6060

61+
test_expect_success POSIXPERM,SANITY 'unwritable rebased-patches does not leak' '
62+
>.git/rebased-patches &&
63+
chmod a-w .git/rebased-patches &&
64+
65+
git checkout -b side HEAD^ &&
66+
test_commit unrelated &&
67+
test_must_fail git rebase --apply --onto tmp HEAD^
68+
'
69+
6170
test_done

0 commit comments

Comments
 (0)