From 63c9bd372e388f5fed77be56771d5ad972f37f8e Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 11 Jun 2024 11:20:42 +0200 Subject: 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 Signed-off-by: Junio C Hamano --- commit.c | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) (limited to 'commit.c') diff --git a/commit.c b/commit.c index 1d08951007..f674eca320 100644 --- a/commit.c +++ b/commit.c @@ -1262,7 +1262,7 @@ int remove_signature(struct strbuf *buf) return sigs[0].start != NULL; } -static void handle_signed_tag(struct commit *parent, struct commit_extra_header ***tail) +static void handle_signed_tag(const struct commit *parent, struct commit_extra_header ***tail) { struct merge_remote_desc *desc; struct commit_extra_header *mergetag; @@ -1359,17 +1359,17 @@ void verify_merge_signature(struct commit *commit, int verbosity, signature_check_clear(&signature_check); } -void append_merge_tag_headers(struct commit_list *parents, +void append_merge_tag_headers(const struct commit_list *parents, struct commit_extra_header ***tail) { while (parents) { - struct commit *parent = parents->item; + const struct commit *parent = parents->item; handle_signed_tag(parent, tail); parents = parents->next; } } -static int convert_commit_extra_headers(struct commit_extra_header *orig, +static int convert_commit_extra_headers(const struct commit_extra_header *orig, struct commit_extra_header **result) { const struct git_hash_algo *compat = the_repository->compat_hash_algo; @@ -1403,7 +1403,7 @@ static int convert_commit_extra_headers(struct commit_extra_header *orig, } static void add_extra_header(struct strbuf *buffer, - struct commit_extra_header *extra) + const struct commit_extra_header *extra) { strbuf_addstr(buffer, extra->key); if (extra->len) @@ -1517,7 +1517,7 @@ void free_commit_extra_headers(struct commit_extra_header *extra) } int commit_tree(const char *msg, size_t msg_len, const struct object_id *tree, - struct commit_list *parents, struct object_id *ret, + const struct commit_list *parents, struct object_id *ret, const char *author, const char *sign_commit) { struct commit_extra_header *extra = NULL, **tail = &extra; @@ -1649,7 +1649,7 @@ static void write_commit_tree(struct strbuf *buffer, const char *msg, size_t msg const struct object_id *tree, const struct object_id *parents, size_t parents_len, const char *author, const char *committer, - struct commit_extra_header *extra) + const struct commit_extra_header *extra) { int encoding_is_utf8; size_t i; @@ -1690,10 +1690,10 @@ static void write_commit_tree(struct strbuf *buffer, const char *msg, size_t msg int commit_tree_extended(const char *msg, size_t msg_len, const struct object_id *tree, - struct commit_list *parents, struct object_id *ret, + const struct commit_list *parents, struct object_id *ret, const char *author, const char *committer, const char *sign_commit, - struct commit_extra_header *extra) + const struct commit_extra_header *extra) { struct repository *r = the_repository; int result = 0; @@ -1715,10 +1715,8 @@ int commit_tree_extended(const char *msg, size_t msg_len, nparents = commit_list_count(parents); CALLOC_ARRAY(parent_buf, nparents); i = 0; - while (parents) { - struct commit *parent = pop_commit(&parents); - oidcpy(&parent_buf[i++], &parent->object.oid); - } + for (const struct commit_list *p = parents; p; p = p->next) + oidcpy(&parent_buf[i++], &p->item->object.oid); write_commit_tree(&buffer, msg, msg_len, tree, parent_buf, nparents, author, committer, extra); if (sign_commit && sign_commit_to_strbuf(&sig, &buffer, sign_commit)) { @@ -1814,7 +1812,7 @@ out: define_commit_slab(merge_desc_slab, struct merge_remote_desc *); static struct merge_desc_slab merge_desc_slab = COMMIT_SLAB_INIT(1, merge_desc_slab); -struct merge_remote_desc *merge_remote_util(struct commit *commit) +struct merge_remote_desc *merge_remote_util(const struct commit *commit) { return *merge_desc_slab_at(&merge_desc_slab, commit); } -- cgit v1.2.3