Merge branch 'ab/sequencer-unleak'

Plug leaks in sequencer subsystem and its users.

* ab/sequencer-unleak:
  commit.c: free() revs.commit in get_fork_point()
  builtin/rebase.c: free() "options.strategy_opts"
  sequencer.c: always free() the "msgbuf" in do_pick_commit()
  builtin/rebase.c: fix "options.onto_name" leak
  builtin/revert.c: move free-ing of "revs" to replay_opts_release()
  sequencer API users: fix get_replay_opts() leaks
  sequencer.c: split up sequencer_remove_state()
  rebase: use "cleanup" pattern in do_interactive_rebase()
This commit is contained in:
Junio C Hamano 2023-02-15 17:11:52 -08:00
commit a232de58f2
23 changed files with 61 additions and 33 deletions

View file

@ -254,7 +254,7 @@ static int init_basic_state(struct replay_opts *opts, const char *head_name,
static int do_interactive_rebase(struct rebase_options *opts, unsigned flags)
{
int ret;
int ret = -1;
char *revisions = NULL, *shortrevisions = NULL;
struct strvec make_script_args = STRVEC_INIT;
struct todo_list todo_list = TODO_LIST_INIT;
@ -262,16 +262,12 @@ static int do_interactive_rebase(struct rebase_options *opts, unsigned flags)
if (get_revision_ranges(opts->upstream, opts->onto, &opts->orig_head->object.oid,
&revisions, &shortrevisions))
return -1;
goto cleanup;
if (init_basic_state(&replay,
opts->head_name ? opts->head_name : "detached HEAD",
opts->onto, &opts->orig_head->object.oid)) {
free(revisions);
free(shortrevisions);
return -1;
}
opts->onto, &opts->orig_head->object.oid))
goto cleanup;
if (!opts->upstream && opts->squash_onto)
write_file(path_squash_onto(), "%s\n",
@ -300,6 +296,8 @@ static int do_interactive_rebase(struct rebase_options *opts, unsigned flags)
opts->autosquash, opts->update_refs, &todo_list);
}
cleanup:
replay_opts_release(&replay);
free(revisions);
free(shortrevisions);
todo_list_release(&todo_list);
@ -341,6 +339,7 @@ static int run_sequencer_rebase(struct rebase_options *opts)
struct replay_opts replay_opts = get_replay_opts(opts);
ret = sequencer_continue(the_repository, &replay_opts);
replay_opts_release(&replay_opts);
break;
}
case ACTION_EDIT_TODO:
@ -556,6 +555,7 @@ static int finish_rebase(struct rebase_options *opts)
replay.action = REPLAY_INTERACTIVE_REBASE;
ret = sequencer_remove_state(&replay);
replay_opts_release(&replay);
} else {
strbuf_addstr(&dir, opts->state_dir);
if (remove_dir_recursively(&dir, 0))
@ -1039,6 +1039,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
struct string_list strategy_options = STRING_LIST_INIT_NODUP;
struct object_id squash_onto;
char *squash_onto_name = NULL;
char *keep_base_onto_name = NULL;
int reschedule_failed_exec = -1;
int allow_preemptive_ff = 1;
int preserve_merges_selected = 0;
@ -1327,6 +1328,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
replay.action = REPLAY_INTERACTIVE_REBASE;
ret = sequencer_remove_state(&replay);
replay_opts_release(&replay);
} else {
strbuf_reset(&buf);
strbuf_addstr(&buf, options.state_dir);
@ -1674,7 +1676,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
strbuf_addstr(&buf, options.upstream_name);
strbuf_addstr(&buf, "...");
strbuf_addstr(&buf, branch_name);
options.onto_name = xstrdup(buf.buf);
options.onto_name = keep_base_onto_name = xstrdup(buf.buf);
} else if (!options.onto_name)
options.onto_name = options.upstream_name;
if (strstr(options.onto_name, "...")) {
@ -1848,8 +1850,10 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
free(options.gpg_sign_opt);
string_list_clear(&options.exec, 0);
free(options.strategy);
free(options.strategy_opts);
strbuf_release(&options.git_format_patch_opt);
free(squash_onto_name);
free(keep_base_onto_name);
string_list_clear(&strategy_options, 0);
return !!ret;
}

View file

@ -248,9 +248,7 @@ int cmd_revert(int argc, const char **argv, const char *prefix)
res = run_sequencer(argc, argv, &opts);
if (res < 0)
die(_("revert failed"));
if (opts.revs)
release_revisions(opts.revs);
free(opts.revs);
replay_opts_release(&opts);
return res;
}
@ -262,10 +260,8 @@ int cmd_cherry_pick(int argc, const char **argv, const char *prefix)
opts.action = REPLAY_PICK;
sequencer_init_config(&opts);
res = run_sequencer(argc, argv, &opts);
if (opts.revs)
release_revisions(opts.revs);
free(opts.revs);
if (res < 0)
die(_("cherry-pick failed"));
replay_opts_release(&opts);
return res;
}

View file

@ -1033,6 +1033,7 @@ struct commit *get_fork_point(const char *refname, struct commit *commit)
ret = bases->item;
cleanup_return:
free(revs.commit);
free_commit_list(bases);
free(full_refname);
return ret;

View file

@ -351,10 +351,25 @@ static const char *gpg_sign_opt_quoted(struct replay_opts *opts)
return buf.buf;
}
void replay_opts_release(struct replay_opts *opts)
{
free(opts->gpg_sign);
free(opts->reflog_action);
free(opts->default_strategy);
free(opts->strategy);
for (size_t i = 0; i < opts->xopts_nr; i++)
free(opts->xopts[i]);
free(opts->xopts);
strbuf_release(&opts->current_fixups);
if (opts->revs)
release_revisions(opts->revs);
free(opts->revs);
}
int sequencer_remove_state(struct replay_opts *opts)
{
struct strbuf buf = STRBUF_INIT;
int i, ret = 0;
int ret = 0;
if (is_rebase_i(opts) &&
strbuf_read_file(&buf, rebase_path_refs_to_delete(), 0) > 0) {
@ -373,15 +388,6 @@ int sequencer_remove_state(struct replay_opts *opts)
}
}
free(opts->gpg_sign);
free(opts->reflog_action);
free(opts->default_strategy);
free(opts->strategy);
for (i = 0; i < opts->xopts_nr; i++)
free(opts->xopts[i]);
free(opts->xopts);
strbuf_release(&opts->current_fixups);
strbuf_reset(&buf);
strbuf_addstr(&buf, get_dir(opts));
if (remove_dir_recursively(&buf, 0))
@ -2271,8 +2277,10 @@ static int do_pick_commit(struct repository *r,
reword = 1;
else if (is_fixup(command)) {
if (update_squash_messages(r, command, commit,
opts, item->flags))
return -1;
opts, item->flags)) {
res = -1;
goto leave;
}
flags |= AMEND_MSG;
if (!final_fixup)
msg_file = rebase_path_squash_msg();
@ -2282,9 +2290,11 @@ static int do_pick_commit(struct repository *r,
} else {
const char *dest = git_path_squash_msg(r);
unlink(dest);
if (copy_file(dest, rebase_path_squash_msg(), 0666))
return error(_("could not rename '%s' to '%s'"),
rebase_path_squash_msg(), dest);
if (copy_file(dest, rebase_path_squash_msg(), 0666)) {
res = error(_("could not rename '%s' to '%s'"),
rebase_path_squash_msg(), dest);
goto leave;
}
unlink(git_path_merge_msg(r));
msg_file = dest;
flags |= EDIT_MSG;
@ -2322,7 +2332,6 @@ static int do_pick_commit(struct repository *r,
free_commit_list(common);
free_commit_list(remotes);
}
strbuf_release(&msgbuf);
/*
* If the merge was clean or if it failed due to conflict, we write
@ -2396,6 +2405,7 @@ static int do_pick_commit(struct repository *r,
leave:
free_message(commit, &msg);
free(author);
strbuf_release(&msgbuf);
update_abort_safety_file();
return res;

View file

@ -158,6 +158,7 @@ int sequencer_pick_revisions(struct repository *repo,
int sequencer_continue(struct repository *repo, struct replay_opts *opts);
int sequencer_rollback(struct repository *repo, struct replay_opts *opts);
int sequencer_skip(struct repository *repo, struct replay_opts *opts);
void replay_opts_release(struct replay_opts *opts);
int sequencer_remove_state(struct replay_opts *opts);
#define TODO_LIST_KEEP_EMPTY (1U << 0)

View file

@ -5,6 +5,7 @@ test_description='rebase should handle arbitrary git message'
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
TEST_PASSES_SANITIZE_LEAK=true
. ./test-lib.sh
. "$TEST_DIRECTORY"/lib-rebase.sh

View file

@ -7,6 +7,7 @@ Tests if git rebase --root --onto <newparent> can rebase the root commit.
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
TEST_PASSES_SANITIZE_LEAK=true
. ./test-lib.sh
log_with_names () {

View file

@ -5,6 +5,7 @@ test_description='git rebase --onto A...B'
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
TEST_PASSES_SANITIZE_LEAK=true
. ./test-lib.sh
. "$TEST_DIRECTORY/lib-rebase.sh"

View file

@ -5,6 +5,7 @@ test_description='git rebase - test patch id computation'
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
TEST_PASSES_SANITIZE_LEAK=true
. ./test-lib.sh
scramble () {

View file

@ -2,6 +2,7 @@
test_description='git rebase interactive with rewording'
TEST_PASSES_SANITIZE_LEAK=true
. ./test-lib.sh
. "$TEST_DIRECTORY"/lib-rebase.sh

View file

@ -1,6 +1,8 @@
#!/bin/sh
test_description='rebase topology tests with merges'
TEST_PASSES_SANITIZE_LEAK=true
. ./test-lib.sh
. "$TEST_DIRECTORY"/lib-rebase.sh

View file

@ -8,6 +8,7 @@ test_description='git rebase --fork-point test'
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
TEST_PASSES_SANITIZE_LEAK=true
. ./test-lib.sh
# A---B---D---E (main)

View file

@ -8,6 +8,7 @@ test_description='ensure rebase fast-forwards commits when possible'
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
TEST_PASSES_SANITIZE_LEAK=true
. ./test-lib.sh
test_expect_success setup '

View file

@ -14,6 +14,7 @@ to the "fixup" command that works with "fixup!", "fixup -C" works with
"amend!" upon --autosquash.
'
TEST_PASSES_SANITIZE_LEAK=true
. ./test-lib.sh
. "$TEST_DIRECTORY"/lib-rebase.sh

View file

@ -1,6 +1,8 @@
#!/bin/sh
test_description='rebase behavior when on-disk files are broken'
TEST_PASSES_SANITIZE_LEAK=true
. ./test-lib.sh
test_expect_success 'set up conflicting branches' '

View file

@ -13,6 +13,7 @@ test_description='test cherry-pick and revert with renames
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
TEST_PASSES_SANITIZE_LEAK=true
. ./test-lib.sh
test_expect_success setup '

View file

@ -11,6 +11,7 @@ test_description='cherry picking and reverting a merge
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
TEST_PASSES_SANITIZE_LEAK=true
. ./test-lib.sh
test_expect_success setup '

View file

@ -5,6 +5,7 @@ test_description='test cherry-picking (and reverting) a root commit'
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
TEST_PASSES_SANITIZE_LEAK=true
. ./test-lib.sh
test_expect_success setup '

View file

@ -5,6 +5,7 @@ test_description='test cherry-picking with --ff option'
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
TEST_PASSES_SANITIZE_LEAK=true
. ./test-lib.sh
test_expect_success setup '

View file

@ -2,6 +2,7 @@
test_description='Test cherry-pick -x and -s'
TEST_PASSES_SANITIZE_LEAK=true
. ./test-lib.sh
pristine_detach () {

View file

@ -5,6 +5,7 @@
test_description='Test rebasing, stashing, etc. with submodules'
TEST_PASSES_SANITIZE_LEAK=true
. ./test-lib.sh
test_expect_success setup '

View file

@ -3,7 +3,6 @@
# Copyright (c) 2006 Eric Wong
test_description='git svn commit-diff clobber'
TEST_FAILS_SANITIZE_LEAK=true
. ./lib-git-svn.sh
test_expect_success 'initialize repo' '

View file

@ -5,7 +5,6 @@
test_description='concurrent git svn dcommit'
TEST_FAILS_SANITIZE_LEAK=true
. ./lib-git-svn.sh