freshlybakedca.ke / git
Git fork
fork atom

sequencer API users: fix get_replay_opts() leaks

Make the replay_opts_release() function added in the preceding commit
non-static, and use it for freeing the "struct replay_opts"
constructed for "rebase" and "revert".

To safely call our new replay_opts_release() we'll need to stop
calling it in sequencer_remove_state(), and instead call it where we
allocate the "struct replay_opts" itself.

This is because in e.g. do_interactive_rebase() we construct a "struct
replay_opts" with "get_replay_opts()", and then call
"complete_action()". If we get far enough in that function without
encountering errors we'll call "pick_commits()" which (indirectly)
calls sequencer_remove_state() at the end.

But if we encounter errors anywhere along the way we'd punt out early,
and not free() the memory we allocated. Remembering whether we
previously called sequencer_remove_state() would be a hassle.

Using a FREE_AND_NULL() pattern would also work, as it would be safe
to call replay_opts_release() repeatedly. But let's fix this properly
instead, by having the owner of the data free() it.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

authored by

Ævar Arnfjörð Bjarmason and committed by
Junio C Hamano 9ff2f060 6a09c3a9

+23 -5
unified split
+4
builtin/rebase.c
··· 297 } 298 299 cleanup: 300 free(revisions); 301 free(shortrevisions); 302 todo_list_release(&todo_list); ··· 338 struct replay_opts replay_opts = get_replay_opts(opts); 339 340 ret = sequencer_continue(the_repository, &replay_opts); 341 break; 342 } 343 case ACTION_EDIT_TODO: ··· 553 554 replay.action = REPLAY_INTERACTIVE_REBASE; 555 ret = sequencer_remove_state(&replay); 556 } else { 557 strbuf_addstr(&dir, opts->state_dir); 558 if (remove_dir_recursively(&dir, 0)) ··· 1324 1325 replay.action = REPLAY_INTERACTIVE_REBASE; 1326 ret = sequencer_remove_state(&replay); 1327 } else { 1328 strbuf_reset(&buf); 1329 strbuf_addstr(&buf, options.state_dir);
··· 297 } 298 299 cleanup: 300 + replay_opts_release(&replay); 301 free(revisions); 302 free(shortrevisions); 303 todo_list_release(&todo_list); ··· 339 struct replay_opts replay_opts = get_replay_opts(opts); 340 341 ret = sequencer_continue(the_repository, &replay_opts); 342 + replay_opts_release(&replay_opts); 343 break; 344 } 345 case ACTION_EDIT_TODO: ··· 555 556 replay.action = REPLAY_INTERACTIVE_REBASE; 557 ret = sequencer_remove_state(&replay); 558 + replay_opts_release(&replay); 559 } else { 560 strbuf_addstr(&dir, opts->state_dir); 561 if (remove_dir_recursively(&dir, 0)) ··· 1327 1328 replay.action = REPLAY_INTERACTIVE_REBASE; 1329 ret = sequencer_remove_state(&replay); 1330 + replay_opts_release(&replay); 1331 } else { 1332 strbuf_reset(&buf); 1333 strbuf_addstr(&buf, options.state_dir);
+2
builtin/revert.c
··· 251 if (opts.revs) 252 release_revisions(opts.revs); 253 free(opts.revs); 254 return res; 255 } 256 ··· 267 free(opts.revs); 268 if (res < 0) 269 die(_("cherry-pick failed")); 270 return res; 271 }
··· 251 if (opts.revs) 252 release_revisions(opts.revs); 253 free(opts.revs); 254 + replay_opts_release(&opts); 255 return res; 256 } 257 ··· 268 free(opts.revs); 269 if (res < 0) 270 die(_("cherry-pick failed")); 271 + replay_opts_release(&opts); 272 return res; 273 }
+1 -3
sequencer.c
··· 351 return buf.buf; 352 } 353 354 - static void replay_opts_release(struct replay_opts *opts) 355 { 356 free(opts->gpg_sign); 357 free(opts->reflog_action); ··· 384 p = eol + 1; 385 } 386 } 387 - 388 - replay_opts_release(opts); 389 390 strbuf_reset(&buf); 391 strbuf_addstr(&buf, get_dir(opts));
··· 351 return buf.buf; 352 } 353 354 + void replay_opts_release(struct replay_opts *opts) 355 { 356 free(opts->gpg_sign); 357 free(opts->reflog_action); ··· 384 p = eol + 1; 385 } 386 } 387 388 strbuf_reset(&buf); 389 strbuf_addstr(&buf, get_dir(opts));
+1
sequencer.h
··· 158 int sequencer_continue(struct repository *repo, struct replay_opts *opts); 159 int sequencer_rollback(struct repository *repo, struct replay_opts *opts); 160 int sequencer_skip(struct repository *repo, struct replay_opts *opts); 161 int sequencer_remove_state(struct replay_opts *opts); 162 163 #define TODO_LIST_KEEP_EMPTY (1U << 0)
··· 158 int sequencer_continue(struct repository *repo, struct replay_opts *opts); 159 int sequencer_rollback(struct repository *repo, struct replay_opts *opts); 160 int sequencer_skip(struct repository *repo, struct replay_opts *opts); 161 + void replay_opts_release(struct replay_opts *opts); 162 int sequencer_remove_state(struct replay_opts *opts); 163 164 #define TODO_LIST_KEEP_EMPTY (1U << 0)
+1
t/t3405-rebase-malformed.sh
··· 5 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main 6 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME 7 8 . ./test-lib.sh 9 . "$TEST_DIRECTORY"/lib-rebase.sh 10
··· 5 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main 6 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME 7 8 + TEST_PASSES_SANITIZE_LEAK=true 9 . ./test-lib.sh 10 . "$TEST_DIRECTORY"/lib-rebase.sh 11
+1
t/t3412-rebase-root.sh
··· 7 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main 8 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME 9 10 . ./test-lib.sh 11 12 log_with_names () {
··· 7 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main 8 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME 9 10 + TEST_PASSES_SANITIZE_LEAK=true 11 . ./test-lib.sh 12 13 log_with_names () {
+1
t/t3419-rebase-patch-id.sh
··· 5 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main 6 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME 7 8 . ./test-lib.sh 9 10 scramble () {
··· 5 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main 6 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME 7 8 + TEST_PASSES_SANITIZE_LEAK=true 9 . ./test-lib.sh 10 11 scramble () {
+1
t/t3423-rebase-reword.sh
··· 2 3 test_description='git rebase interactive with rewording' 4 5 . ./test-lib.sh 6 7 . "$TEST_DIRECTORY"/lib-rebase.sh
··· 2 3 test_description='git rebase interactive with rewording' 4 5 + TEST_PASSES_SANITIZE_LEAK=true 6 . ./test-lib.sh 7 8 . "$TEST_DIRECTORY"/lib-rebase.sh
+2
t/t3425-rebase-topology-merges.sh
··· 1 #!/bin/sh 2 3 test_description='rebase topology tests with merges' 4 . ./test-lib.sh 5 . "$TEST_DIRECTORY"/lib-rebase.sh 6
··· 1 #!/bin/sh 2 3 test_description='rebase topology tests with merges' 4 + 5 + TEST_PASSES_SANITIZE_LEAK=true 6 . ./test-lib.sh 7 . "$TEST_DIRECTORY"/lib-rebase.sh 8
+1
t/t3437-rebase-fixup-options.sh
··· 14 "amend!" upon --autosquash. 15 ' 16 17 . ./test-lib.sh 18 19 . "$TEST_DIRECTORY"/lib-rebase.sh
··· 14 "amend!" upon --autosquash. 15 ' 16 17 + TEST_PASSES_SANITIZE_LEAK=true 18 . ./test-lib.sh 19 20 . "$TEST_DIRECTORY"/lib-rebase.sh
+2
t/t3438-rebase-broken-files.sh
··· 1 #!/bin/sh 2 3 test_description='rebase behavior when on-disk files are broken' 4 . ./test-lib.sh 5 6 test_expect_success 'set up conflicting branches' '
··· 1 #!/bin/sh 2 3 test_description='rebase behavior when on-disk files are broken' 4 + 5 + TEST_PASSES_SANITIZE_LEAK=true 6 . ./test-lib.sh 7 8 test_expect_success 'set up conflicting branches' '
+1
t/t3501-revert-cherry-pick.sh
··· 13 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main 14 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME 15 16 . ./test-lib.sh 17 18 test_expect_success setup '
··· 13 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main 14 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME 15 16 + TEST_PASSES_SANITIZE_LEAK=true 17 . ./test-lib.sh 18 19 test_expect_success setup '
+1
t/t3502-cherry-pick-merge.sh
··· 11 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main 12 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME 13 14 . ./test-lib.sh 15 16 test_expect_success setup '
··· 11 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main 12 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME 13 14 + TEST_PASSES_SANITIZE_LEAK=true 15 . ./test-lib.sh 16 17 test_expect_success setup '
+1
t/t3503-cherry-pick-root.sh
··· 5 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main 6 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME 7 8 . ./test-lib.sh 9 10 test_expect_success setup '
··· 5 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main 6 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME 7 8 + TEST_PASSES_SANITIZE_LEAK=true 9 . ./test-lib.sh 10 11 test_expect_success setup '
+1
t/t3506-cherry-pick-ff.sh
··· 5 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main 6 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME 7 8 . ./test-lib.sh 9 10 test_expect_success setup '
··· 5 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main 6 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME 7 8 + TEST_PASSES_SANITIZE_LEAK=true 9 . ./test-lib.sh 10 11 test_expect_success setup '
+1
t/t3511-cherry-pick-x.sh
··· 2 3 test_description='Test cherry-pick -x and -s' 4 5 . ./test-lib.sh 6 7 pristine_detach () {
··· 2 3 test_description='Test cherry-pick -x and -s' 4 5 + TEST_PASSES_SANITIZE_LEAK=true 6 . ./test-lib.sh 7 8 pristine_detach () {
+1
t/t7402-submodule-rebase.sh
··· 5 6 test_description='Test rebasing, stashing, etc. with submodules' 7 8 . ./test-lib.sh 9 10 test_expect_success setup '
··· 5 6 test_description='Test rebasing, stashing, etc. with submodules' 7 8 + TEST_PASSES_SANITIZE_LEAK=true 9 . ./test-lib.sh 10 11 test_expect_success setup '
-1
t/t9106-git-svn-commit-diff-clobber.sh
··· 3 # Copyright (c) 2006 Eric Wong 4 test_description='git svn commit-diff clobber' 5 6 - TEST_FAILS_SANITIZE_LEAK=true 7 . ./lib-git-svn.sh 8 9 test_expect_success 'initialize repo' '
··· 3 # Copyright (c) 2006 Eric Wong 4 test_description='git svn commit-diff clobber' 5 6 . ./lib-git-svn.sh 7 8 test_expect_success 'initialize repo' '
-1
t/t9164-git-svn-dcommit-concurrent.sh
··· 5 6 test_description='concurrent git svn dcommit' 7 8 - TEST_FAILS_SANITIZE_LEAK=true 9 . ./lib-git-svn.sh 10 11
··· 5 6 test_description='concurrent git svn dcommit' 7 8 . ./lib-git-svn.sh 9 10