aboutsummaryrefslogtreecommitdiffstats
path: root/builtin
diff options
context:
space:
mode:
authorPatrick Steinhardt <ps@pks.im>2024-06-11 11:20:42 +0200
committerJunio C Hamano <gitster@pobox.com>2024-06-11 13:15:07 -0700
commit63c9bd372e388f5fed77be56771d5ad972f37f8e (patch)
tree70f1e32515bb7b1142f310ef9995a30e19859044 /builtin
parentc6eb58bfb19a0840841934b91613ea53f0da8651 (diff)
downloadgit-63c9bd372e388f5fed77be56771d5ad972f37f8e.tar.gz
commit: fix leaking parents when calling `commit_tree_extended()`
When creating commits via `commit_tree_extended()`, the caller passes in a string list of parents. This call implicitly transfers ownership of that list to the function, which is quite surprising to begin with. But to make matters worse, `commit_tree_extended()` doesn't even bother to free the list of parents in error cases. The result is a memory leak, and one that the caller cannot fix by themselves because they do not know whether parts of the string list have already been released. Refactor the code such that callers can keep ownership of the list of parents, which is getting indicated by parameter being a constant pointer now. Free the lists at the calling site and add a common exit path to those sites as required. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Diffstat (limited to 'builtin')
-rw-r--r--builtin/am.c1
-rw-r--r--builtin/commit-tree.c11
-rw-r--r--builtin/commit.c3
-rw-r--r--builtin/merge.c6
-rw-r--r--builtin/replay.c14
-rw-r--r--builtin/stash.c9
6 files changed, 29 insertions, 15 deletions
diff --git a/builtin/am.c b/builtin/am.c
index 4ba44e2d70..faccc45b13 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1718,6 +1718,7 @@ static void do_commit(const struct am_state *state)
run_hooks("post-applypatch");
+ free_commit_list(parents);
strbuf_release(&sb);
}
diff --git a/builtin/commit-tree.c b/builtin/commit-tree.c
index 1bb7819839..84bb450222 100644
--- a/builtin/commit-tree.c
+++ b/builtin/commit-tree.c
@@ -111,6 +111,7 @@ int cmd_commit_tree(int argc, const char **argv, const char *prefix)
N_("GPG sign commit"), PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
OPT_END()
};
+ int ret;
git_config(git_default_config, NULL);
@@ -132,11 +133,15 @@ int cmd_commit_tree(int argc, const char **argv, const char *prefix)
if (commit_tree(buffer.buf, buffer.len, &tree_oid, parents, &commit_oid,
NULL, sign_commit)) {
- strbuf_release(&buffer);
- return 1;
+ ret = 1;
+ goto out;
}
printf("%s\n", oid_to_hex(&commit_oid));
+ ret = 0;
+
+out:
+ free_commit_list(parents);
strbuf_release(&buffer);
- return 0;
+ return ret;
}
diff --git a/builtin/commit.c b/builtin/commit.c
index dcaf4efa03..d5713455e5 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1848,7 +1848,6 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
rollback_index_files();
die(_("failed to write commit object"));
}
- free_commit_extra_headers(extra);
if (update_head_with_reflog(current_head, &oid, reflog_msg, &sb,
&err)) {
@@ -1890,6 +1889,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
apply_autostash_ref(the_repository, "MERGE_AUTOSTASH");
cleanup:
+ free_commit_extra_headers(extra);
+ free_commit_list(parents);
strbuf_release(&author_ident);
strbuf_release(&err);
strbuf_release(&sb);
diff --git a/builtin/merge.c b/builtin/merge.c
index daed2d4e1e..50b0c87a95 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -895,7 +895,7 @@ static void prepare_to_commit(struct commit_list *remoteheads)
static int merge_trivial(struct commit *head, struct commit_list *remoteheads)
{
struct object_id result_tree, result_commit;
- struct commit_list *parents, **pptr = &parents;
+ struct commit_list *parents = NULL, **pptr = &parents;
if (repo_refresh_and_write_index(the_repository, REFRESH_QUIET,
SKIP_IF_UNCHANGED, 0, NULL, NULL,
@@ -911,7 +911,9 @@ static int merge_trivial(struct commit *head, struct commit_list *remoteheads)
&result_commit, NULL, sign_commit))
die(_("failed to write commit object"));
finish(head, remoteheads, &result_commit, "In-index merge");
+
remove_merge_branch_state(the_repository);
+ free_commit_list(parents);
return 0;
}
@@ -937,8 +939,10 @@ static int finish_automerge(struct commit *head,
die(_("failed to write commit object"));
strbuf_addf(&buf, "Merge made by the '%s' strategy.", wt_strategy);
finish(head, remoteheads, &result_commit, buf.buf);
+
strbuf_release(&buf);
remove_merge_branch_state(the_repository);
+ free_commit_list(parents);
return 0;
}
diff --git a/builtin/replay.c b/builtin/replay.c
index 6bf0691f15..0448326636 100644
--- a/builtin/replay.c
+++ b/builtin/replay.c
@@ -52,11 +52,11 @@ static struct commit *create_commit(struct tree *tree,
struct commit *parent)
{
struct object_id ret;
- struct object *obj;
+ struct object *obj = NULL;
struct commit_list *parents = NULL;
char *author;
char *sign_commit = NULL; /* FIXME: cli users might want to sign again */
- struct commit_extra_header *extra;
+ struct commit_extra_header *extra = NULL;
struct strbuf msg = STRBUF_INIT;
const char *out_enc = get_commit_output_encoding();
const char *message = repo_logmsg_reencode(the_repository, based_on,
@@ -73,12 +73,16 @@ static struct commit *create_commit(struct tree *tree,
if (commit_tree_extended(msg.buf, msg.len, &tree->object.oid, parents,
&ret, author, NULL, sign_commit, extra)) {
error(_("failed to write commit object"));
- return NULL;
+ goto out;
}
- free(author);
- strbuf_release(&msg);
obj = parse_object(the_repository, &ret);
+
+out:
+ free_commit_extra_headers(extra);
+ free_commit_list(parents);
+ strbuf_release(&msg);
+ free(author);
return (struct commit *)obj;
}
diff --git a/builtin/stash.c b/builtin/stash.c
index 1ed0a9a5d9..46b981c4dd 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -1416,6 +1416,9 @@ static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_b
goto done;
}
+ free_commit_list(parents);
+ parents = NULL;
+
if (include_untracked) {
if (save_untracked_files(info, &msg, untracked_files)) {
if (!quiet)
@@ -1461,11 +1464,6 @@ static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_b
else
strbuf_insertf(stash_msg_buf, 0, "On %s: ", branch_name);
- /*
- * `parents` will be empty after calling `commit_tree()`, so there is
- * no need to call `free_commit_list()`
- */
- parents = NULL;
if (untracked_commit_option)
commit_list_insert(lookup_commit(the_repository,
&info->u_commit),
@@ -1487,6 +1485,7 @@ done:
strbuf_release(&commit_tree_label);
strbuf_release(&msg);
strbuf_release(&untracked_files);
+ free_commit_list(parents);
return ret;
}
generated by cgit 1.2.3-korg (git 2.43.0) at 2025-12-07 09:35:53 +0000