From a9539a993a2b4dbfb5540aebb02bfcfd5be4c24f Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 5 Sep 2024 12:08:38 +0200 Subject: [PATCH 01/22] t/test-lib: allow skipping leak checks for passing tests With `GIT_TEST_PASSING_SANITIZE_LEAK=check`, one can double check whether a memory leak fix caused some test suites to become leak free. This is done by running all tests with the leak checker enabled. If a test suite does not declare `TEST_PASSES_SANITIZE_LEAK=true` but still finishes successfully with the leak checker enabled, then this indicates that the test is leak free and thus missing the annotation. It is somewhat slow to execute though because it runs all of our test suites with the leak sanitizer enabled. It is also pointless in most cases, because the only test suites that need to be checked are those which _aren't_ yet marked with `TEST_PASSES_SANITIZE_LEAK=true`. Introduce a new value "check-failing". When set, we behave the same as if "check" was passed, except that we only check those tests which do not have `TEST_PASSES_SANITIZE_LEAK=true` set. This is significantly faster than running all test suites but still fulfills the usecase of finding newly-leak-free test suites. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- t/README | 3 +++ t/test-lib.sh | 11 ++++++++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/t/README b/t/README index 44c02d8129..8dcb778e26 100644 --- a/t/README +++ b/t/README @@ -386,6 +386,9 @@ GIT_TEST_PASSING_SANITIZE_LEAK=check when combined with "--immediate" will run to completion faster, and result in the same failing tests. +GIT_TEST_PASSING_SANITIZE_LEAK=check-failing behaves the same as "check", +but skips all tests which are already marked as leak-free. + GIT_TEST_PROTOCOL_VERSION=, when set, makes 'protocol.version' default to n. diff --git a/t/test-lib.sh b/t/test-lib.sh index 54247604cb..64bd36531c 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -1558,8 +1558,16 @@ then passes_sanitize_leak=t fi - if test "$GIT_TEST_PASSING_SANITIZE_LEAK" = "check" + if test "$GIT_TEST_PASSING_SANITIZE_LEAK" = "check" || + test "$GIT_TEST_PASSING_SANITIZE_LEAK" = "check-failing" then + if test "$GIT_TEST_PASSING_SANITIZE_LEAK" = "check-failing" && + test -n "$passes_sanitize_leak" + then + skip_all="skipping leak-free $this_test under GIT_TEST_PASSING_SANITIZE_LEAK=check-failing" + test_done + fi + sanitize_leak_check=t if test -n "$invert_exit_code" then @@ -1597,6 +1605,7 @@ then export LSAN_OPTIONS elif test "$GIT_TEST_PASSING_SANITIZE_LEAK" = "check" || + test "$GIT_TEST_PASSING_SANITIZE_LEAK" = "check-failing" || test_bool_env GIT_TEST_PASSING_SANITIZE_LEAK false then BAIL_OUT_ENV_NEEDS_SANITIZE_LEAK "GIT_TEST_PASSING_SANITIZE_LEAK=true" From 63494913eced2f0993eb431ad236b03e6ee8cac2 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 5 Sep 2024 12:08:40 +0200 Subject: [PATCH 02/22] fetch-pack: fix memory leaks on fetch negotiation We leak both the `nt_object_array` and `negotiator` structures in `negotiate_using_fetch()`. Plug both of these leaks. These leaks were exposed by t5516, but fixing them is not sufficient to make the whole test suite leak free. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- fetch-pack.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/fetch-pack.c b/fetch-pack.c index 58b4581ad8..0ed82feda1 100644 --- a/fetch-pack.c +++ b/fetch-pack.c @@ -2227,7 +2227,10 @@ void negotiate_using_fetch(const struct oid_array *negotiation_tips, trace2_region_leave("fetch-pack", "negotiate_using_fetch", the_repository); trace2_data_intmax("negotiate_using_fetch", the_repository, "total_rounds", negotiation_round); + clear_common_flag(acked_commits); + object_array_clear(&nt_object_array); + negotiator.release(&negotiator); strbuf_release(&req_buf); } From e03004f7f86a817af2b8d0752dfecac58e7d85e0 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 5 Sep 2024 12:08:43 +0200 Subject: [PATCH 03/22] send-pack: fix leaking common object IDs We're leaking the array of common object IDs in `send_pack()`. Fix this by creating a common exit path where we free the leaking data. While at it, unify some other cleanups now that we have a central place to put them. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- send-pack.c | 34 ++++++++++++++++++++++------------ t/t5549-fetch-push-http.sh | 1 + 2 files changed, 23 insertions(+), 12 deletions(-) diff --git a/send-pack.c b/send-pack.c index fa2f5eec17..b224ef9fc5 100644 --- a/send-pack.c +++ b/send-pack.c @@ -508,7 +508,8 @@ int send_pack(struct send_pack_args *args, if (!remote_refs) { fprintf(stderr, "No refs in common and none specified; doing nothing.\n" "Perhaps you should specify a branch.\n"); - return 0; + ret = 0; + goto out; } git_config_get_bool("push.negotiate", &push_negotiate); @@ -615,12 +616,11 @@ int send_pack(struct send_pack_args *args, * atomically, abort the whole operation. */ if (use_atomic) { - strbuf_release(&req_buf); - strbuf_release(&cap_buf); reject_atomic_push(remote_refs, args->send_mirror); error("atomic push failed for ref %s. status: %d\n", ref->name, ref->status); - return args->porcelain ? 0 : -1; + ret = args->porcelain ? 0 : -1; + goto out; } /* else fallthrough */ default: @@ -682,8 +682,6 @@ int send_pack(struct send_pack_args *args, write_or_die(out, req_buf.buf, req_buf.len); packet_flush(out); } - strbuf_release(&req_buf); - strbuf_release(&cap_buf); if (use_sideband && cmds_sent) { memset(&demux, 0, sizeof(demux)); @@ -721,7 +719,9 @@ int send_pack(struct send_pack_args *args, finish_async(&demux); } fd[1] = -1; - return -1; + + ret = -1; + goto out; } if (!args->stateless_rpc) /* Closed by pack_objects() via start_command() */ @@ -746,10 +746,12 @@ int send_pack(struct send_pack_args *args, } if (ret < 0) - return ret; + goto out; - if (args->porcelain) - return 0; + if (args->porcelain) { + ret = 0; + goto out; + } for (ref = remote_refs; ref; ref = ref->next) { switch (ref->status) { @@ -758,8 +760,16 @@ int send_pack(struct send_pack_args *args, case REF_STATUS_OK: break; default: - return -1; + ret = -1; + goto out; } } - return 0; + + ret = 0; + +out: + oid_array_clear(&commons); + strbuf_release(&req_buf); + strbuf_release(&cap_buf); + return ret; } diff --git a/t/t5549-fetch-push-http.sh b/t/t5549-fetch-push-http.sh index 2cdebcb735..6377fb6d99 100755 --- a/t/t5549-fetch-push-http.sh +++ b/t/t5549-fetch-push-http.sh @@ -5,6 +5,7 @@ test_description='fetch/push functionality using the HTTP protocol' 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-httpd.sh start_httpd From 7eb6f02c554b2e41f4b33152163868a84a0afa85 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 5 Sep 2024 12:08:46 +0200 Subject: [PATCH 04/22] builtin/push: fix leaking refspec query result When appending a refspec via `refspec_append_mapped()` we leak the result of `query_refspecs()`. The overall logic around refspec queries is quite weird, as callers are expected to either set the `src` or `dst` pointers, and then the (allocated) result will be in the respective other struct member. As we have the `src` member set, plugging the memory leak is thus as easy as just freeing the `dst` member. While at it, use designated initializers to initialize the structure. This leak was exposed by t5516, but fixing it is not sufficient to make the whole test suite leak free. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/push.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/builtin/push.c b/builtin/push.c index 7a67398124..0b123eb9c1 100644 --- a/builtin/push.c +++ b/builtin/push.c @@ -72,13 +72,15 @@ static void refspec_append_mapped(struct refspec *refspec, const char *ref, const char *branch_name; if (remote->push.nr) { - struct refspec_item query; - memset(&query, 0, sizeof(struct refspec_item)); - query.src = matched->name; + struct refspec_item query = { + .src = matched->name, + }; + if (!query_refspecs(&remote->push, &query) && query.dst) { refspec_appendf(refspec, "%s%s:%s", query.force ? "+" : "", query.src, query.dst); + free(query.dst); return; } } From ac2e7d545efdc4ceeef8c1191bb276e86d793f29 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 5 Sep 2024 12:08:48 +0200 Subject: [PATCH 05/22] upload-pack: fix leaking child process data on reachability checks We spawn a git-rev-list(1) command to perform reachability checks in "upload-pack.c". We do not release memory associated with the process in error cases though, thus leaking memory. Fix these by calling `child_process_clear()`. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- t/t5516-fetch-push.sh | 1 + upload-pack.c | 22 ++++++++++++++++------ 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh index 9d693eb57f..331778bd42 100755 --- a/t/t5516-fetch-push.sh +++ b/t/t5516-fetch-push.sh @@ -19,6 +19,7 @@ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME TEST_CREATE_REPO_NO_TEMPLATE=1 +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh D=$(pwd) diff --git a/upload-pack.c b/upload-pack.c index f03ba3e98b..c84c3c3b1f 100644 --- a/upload-pack.c +++ b/upload-pack.c @@ -709,10 +709,13 @@ static int get_reachable_list(struct upload_pack_data *data, struct object *o; char namebuf[GIT_MAX_HEXSZ + 2]; /* ^ + hash + LF */ const unsigned hexsz = the_hash_algo->hexsz; + int ret; if (do_reachable_revlist(&cmd, &data->shallows, reachable, - data->allow_uor) < 0) - return -1; + data->allow_uor) < 0) { + ret = -1; + goto out; + } while ((i = read_in_full(cmd.out, namebuf, hexsz + 1)) == hexsz + 1) { struct object_id oid; @@ -736,10 +739,16 @@ static int get_reachable_list(struct upload_pack_data *data, } close(cmd.out); - if (finish_command(&cmd)) - return -1; + if (finish_command(&cmd)) { + ret = -1; + goto out; + } - return 0; + ret = 0; + +out: + child_process_clear(&cmd); + return ret; } static int has_unreachable(struct object_array *src, enum allow_uor allow_uor) @@ -749,7 +758,7 @@ static int has_unreachable(struct object_array *src, enum allow_uor allow_uor) int i; if (do_reachable_revlist(&cmd, src, NULL, allow_uor) < 0) - return 1; + goto error; /* * The commits out of the rev-list are not ancestors of @@ -775,6 +784,7 @@ static int has_unreachable(struct object_array *src, enum allow_uor allow_uor) error: if (cmd.out >= 0) close(cmd.out); + child_process_clear(&cmd); return 1; } From 3eefd348e5865b0406d32194a157ae77d9a7939b Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 5 Sep 2024 12:08:53 +0200 Subject: [PATCH 06/22] submodule: fix leaking fetch task data The `submodule_parallel_fetch` structure contains various data structures that we use to set up parallel fetches of submodules. We do not free some of its data though, causing memory leaks. Plug those. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- submodule.c | 2 ++ t/t5526-fetch-submodules.sh | 1 + 2 files changed, 3 insertions(+) diff --git a/submodule.c b/submodule.c index 97516b0fec..97d0d47b56 100644 --- a/submodule.c +++ b/submodule.c @@ -1883,6 +1883,8 @@ int fetch_submodules(struct repository *r, out: free_submodules_data(&spf.changed_submodule_names); string_list_clear(&spf.seen_submodule_names, 0); + strbuf_release(&spf.submodules_with_errors); + free(spf.oid_fetch_tasks); return spf.result; } diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh index 5e566205ba..2cfb5bd6bb 100755 --- a/t/t5526-fetch-submodules.sh +++ b/t/t5526-fetch-submodules.sh @@ -6,6 +6,7 @@ test_description='Recursive "git fetch" for submodules' GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB=1 export GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh pwd=$(pwd) From 1e8cb17ac5c0e7871e15dd40bacd617f25a54763 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 5 Sep 2024 12:08:56 +0200 Subject: [PATCH 07/22] builtin/submodule--helper: fix leaking refs on push-check In the push-check subcommand of the submodule helper we acquire a list of local refs, but never free that list. Fix this memory leak. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/submodule--helper.c | 2 ++ t/t5531-deep-submodule-push.sh | 1 + 2 files changed, 3 insertions(+) diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 85fb23dee8..642a0edabf 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -2958,7 +2958,9 @@ static int push_check(int argc, const char **argv, const char *prefix UNUSED) rs->src); } } + refspec_clear(&refspec); + free_refs(local_refs); } free(head); diff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh index f3fff55744..135823630a 100755 --- a/t/t5531-deep-submodule-push.sh +++ b/t/t5531-deep-submodule-push.sh @@ -8,6 +8,7 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB=1 export GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh test_expect_success setup ' From cdbb7208c80661754b3f1a73aef086c08f88dcff Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 5 Sep 2024 12:08:59 +0200 Subject: [PATCH 08/22] remote: fix leaking tracking refs When computing the remote tracking ref we cause two memory leaks: - We leak when `remote_tracking()` fails. - We leak when the call to `remote_tracking()` succeeds and sets `ref->tracking_ref()`. Fix both of these leaks. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- remote.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/remote.c b/remote.c index 7d5b8f750d..8d666c1641 100644 --- a/remote.c +++ b/remote.c @@ -1097,6 +1097,7 @@ void free_one_ref(struct ref *ref) return; free_one_ref(ref->peer_ref); free(ref->remote_status); + free(ref->tracking_ref); free(ref->symref); free(ref); } @@ -2577,8 +2578,10 @@ static int remote_tracking(struct remote *remote, const char *refname, dst = apply_refspecs(&remote->fetch, refname); if (!dst) return -1; /* no tracking ref for refname at remote */ - if (refs_read_ref(get_main_ref_store(the_repository), dst, oid)) + if (refs_read_ref(get_main_ref_store(the_repository), dst, oid)) { + free(dst); return -1; /* we know what the tracking ref is but we cannot read it */ + } *dst_refname = dst; return 0; From 42c153e1c06fcfea8446f11bf3fc3bcf9ea25867 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 5 Sep 2024 12:09:01 +0200 Subject: [PATCH 09/22] remote: fix leak in reachability check of a remote-tracking ref In `check_if_includes_upstream()` we retrieve the local ref corresponding to a remote-tracking ref we want to check reachability for. We never free that local ref and thus cause a memory leak. Fix this. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- remote.c | 1 + t/t5533-push-cas.sh | 1 + 2 files changed, 2 insertions(+) diff --git a/remote.c b/remote.c index 8d666c1641..e11b03a075 100644 --- a/remote.c +++ b/remote.c @@ -2731,6 +2731,7 @@ static void check_if_includes_upstream(struct ref *remote) if (is_reachable_in_reflog(local->name, remote) <= 0) remote->unreachable = 1; + free_one_ref(local); } static void apply_cas(struct push_cas_option *cas, diff --git a/t/t5533-push-cas.sh b/t/t5533-push-cas.sh index cba26a872d..6365d99777 100755 --- a/t/t5533-push-cas.sh +++ b/t/t5533-push-cas.sh @@ -5,6 +5,7 @@ test_description='compare & swap push force/delete safety' GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh setup_srcdst_basic () { From 49d47eb5416d22f185877a57380a1ffc28f172e1 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 5 Sep 2024 12:09:04 +0200 Subject: [PATCH 10/22] send-pack: fix leaking push cert nonce When retrieving the push cert nonce from the server, we first store the constant returned by `server_feature_value()` and then, if the nonce is valid, we duplicate the nonce memory to a NUL-terminated string, so that we can pass it to `generate_push_cert()`. We never free the latter and thus cause a memory leak. Fix this by storing the limited-lifetime nonce into a scope-local variable such that the long-lived, allocated nonce can be easily freed without having to cast away its constness. This leak was exposed by t5534, but fixing it is not sufficient to make the whole test suite leak free. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- send-pack.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/send-pack.c b/send-pack.c index b224ef9fc5..c37f6ab3c0 100644 --- a/send-pack.c +++ b/send-pack.c @@ -501,7 +501,7 @@ int send_pack(struct send_pack_args *args, unsigned cmds_sent = 0; int ret; struct async demux; - const char *push_cert_nonce = NULL; + char *push_cert_nonce = NULL; struct packet_reader reader; int use_bitmaps; @@ -550,10 +550,11 @@ int send_pack(struct send_pack_args *args, if (args->push_cert != SEND_PACK_PUSH_CERT_NEVER) { size_t len; - push_cert_nonce = server_feature_value("push-cert", &len); - if (push_cert_nonce) { - reject_invalid_nonce(push_cert_nonce, len); - push_cert_nonce = xmemdupz(push_cert_nonce, len); + const char *nonce = server_feature_value("push-cert", &len); + + if (nonce) { + reject_invalid_nonce(nonce, len); + push_cert_nonce = xmemdupz(nonce, len); } else if (args->push_cert == SEND_PACK_PUSH_CERT_ALWAYS) { die(_("the receiving end does not support --signed push")); } else if (args->push_cert == SEND_PACK_PUSH_CERT_IF_ASKED) { @@ -771,5 +772,6 @@ int send_pack(struct send_pack_args *args, oid_array_clear(&commons); strbuf_release(&req_buf); strbuf_release(&cap_buf); + free(push_cert_nonce); return ret; } From b8849e236f7a32d43ab3ba087587a336d69329b0 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 5 Sep 2024 12:09:07 +0200 Subject: [PATCH 11/22] gpg-interface: fix misdesigned signing key interfaces The interfaces to retrieve signing keys and their IDs are misdesigned as they return string constants even though they indeed allocate memory, which leads to memory leaks. Refactor the code to instead always return allocated strings and let the callers free them accordingly. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/tag.c | 3 ++- commit.c | 9 ++++++--- gpg-interface.c | 26 +++++++++++++++----------- gpg-interface.h | 4 ++-- send-pack.c | 6 ++++-- t/t5534-push-signed.sh | 1 + 6 files changed, 30 insertions(+), 19 deletions(-) diff --git a/builtin/tag.c b/builtin/tag.c index a1fb218512..ab3b500543 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -160,7 +160,7 @@ static int do_sign(struct strbuf *buffer, struct object_id **compat_oid, const struct git_hash_algo *compat = the_repository->compat_hash_algo; struct strbuf sig = STRBUF_INIT, compat_sig = STRBUF_INIT; struct strbuf compat_buf = STRBUF_INIT; - const char *keyid = get_signing_key(); + char *keyid = get_signing_key(); int ret = -1; if (sign_buffer(buffer, &sig, keyid)) @@ -190,6 +190,7 @@ static int do_sign(struct strbuf *buffer, struct object_id **compat_oid, strbuf_release(&sig); strbuf_release(&compat_sig); strbuf_release(&compat_buf); + free(keyid); return ret; } diff --git a/commit.c b/commit.c index 24ab5c1b50..ec9efc189d 100644 --- a/commit.c +++ b/commit.c @@ -1150,11 +1150,14 @@ int add_header_signature(struct strbuf *buf, struct strbuf *sig, const struct gi static int sign_commit_to_strbuf(struct strbuf *sig, struct strbuf *buf, const char *keyid) { + char *keyid_to_free = NULL; + int ret = 0; if (!keyid || !*keyid) - keyid = get_signing_key(); + keyid = keyid_to_free = get_signing_key(); if (sign_buffer(buf, sig, keyid)) - return -1; - return 0; + ret = -1; + free(keyid_to_free); + return ret; } int parse_signed_commit(const struct commit *commit, diff --git a/gpg-interface.c b/gpg-interface.c index 6587085cd1..cf6126b5aa 100644 --- a/gpg-interface.c +++ b/gpg-interface.c @@ -45,8 +45,8 @@ struct gpg_format { size_t signature_size); int (*sign_buffer)(struct strbuf *buffer, struct strbuf *signature, const char *signing_key); - const char *(*get_default_key)(void); - const char *(*get_key_id)(void); + char *(*get_default_key)(void); + char *(*get_key_id)(void); }; static const char *openpgp_verify_args[] = { @@ -86,9 +86,9 @@ static int sign_buffer_gpg(struct strbuf *buffer, struct strbuf *signature, static int sign_buffer_ssh(struct strbuf *buffer, struct strbuf *signature, const char *signing_key); -static const char *get_default_ssh_signing_key(void); +static char *get_default_ssh_signing_key(void); -static const char *get_ssh_key_id(void); +static char *get_ssh_key_id(void); static struct gpg_format gpg_format[] = { { @@ -847,7 +847,7 @@ static char *get_ssh_key_fingerprint(const char *signing_key) } /* Returns the first public key from an ssh-agent to use for signing */ -static const char *get_default_ssh_signing_key(void) +static char *get_default_ssh_signing_key(void) { struct child_process ssh_default_key = CHILD_PROCESS_INIT; int ret = -1; @@ -899,12 +899,16 @@ static const char *get_default_ssh_signing_key(void) return default_key; } -static const char *get_ssh_key_id(void) { - return get_ssh_key_fingerprint(get_signing_key()); +static char *get_ssh_key_id(void) +{ + char *signing_key = get_signing_key(); + char *key_id = get_ssh_key_fingerprint(signing_key); + free(signing_key); + return key_id; } /* Returns a textual but unique representation of the signing key */ -const char *get_signing_key_id(void) +char *get_signing_key_id(void) { gpg_interface_lazy_init(); @@ -916,17 +920,17 @@ const char *get_signing_key_id(void) return get_signing_key(); } -const char *get_signing_key(void) +char *get_signing_key(void) { gpg_interface_lazy_init(); if (configured_signing_key) - return configured_signing_key; + return xstrdup(configured_signing_key); if (use_format->get_default_key) { return use_format->get_default_key(); } - return git_committer_info(IDENT_STRICT | IDENT_NO_DATE); + return xstrdup(git_committer_info(IDENT_STRICT | IDENT_NO_DATE)); } const char *gpg_trust_level_to_str(enum signature_trust_level level) diff --git a/gpg-interface.h b/gpg-interface.h index 7cd98161f7..e09f12e8d0 100644 --- a/gpg-interface.h +++ b/gpg-interface.h @@ -80,13 +80,13 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *gpg_trust_level_to_str(enum signature_trust_level level); void set_signing_key(const char *); -const char *get_signing_key(void); +char *get_signing_key(void); /* * Returns a textual unique representation of the signing key in use * Either a GPG KeyID or a SSH Key Fingerprint */ -const char *get_signing_key_id(void); +char *get_signing_key_id(void); int check_signature(struct signature_check *sigc, const char *signature, size_t slen); void print_signature_buffer(const struct signature_check *sigc, diff --git a/send-pack.c b/send-pack.c index c37f6ab3c0..31a62e6a98 100644 --- a/send-pack.c +++ b/send-pack.c @@ -348,7 +348,8 @@ static int generate_push_cert(struct strbuf *req_buf, { const struct ref *ref; struct string_list_item *item; - char *signing_key_id = xstrdup(get_signing_key_id()); + char *signing_key_id = get_signing_key_id(); + char *signing_key = get_signing_key(); const char *cp, *np; struct strbuf cert = STRBUF_INIT; int update_seen = 0; @@ -381,7 +382,7 @@ static int generate_push_cert(struct strbuf *req_buf, if (!update_seen) goto free_return; - if (sign_buffer(&cert, &cert, get_signing_key())) + if (sign_buffer(&cert, &cert, signing_key)) die(_("failed to sign the push certificate")); packet_buf_write(req_buf, "push-cert%c%s", 0, cap_string); @@ -394,6 +395,7 @@ static int generate_push_cert(struct strbuf *req_buf, free_return: free(signing_key_id); + free(signing_key); strbuf_release(&cert); return update_seen; } diff --git a/t/t5534-push-signed.sh b/t/t5534-push-signed.sh index c91a62b77a..d43aee0c32 100755 --- a/t/t5534-push-signed.sh +++ b/t/t5534-push-signed.sh @@ -5,6 +5,7 @@ test_description='signed push' 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-gpg.sh From 0d1d22f5a385d05bde40303c17483db2eec499b3 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 5 Sep 2024 12:09:12 +0200 Subject: [PATCH 12/22] object: clear grafts when clearing parsed object pool We do not clear grafts part of the parsed object pool when clearing the pool itself, which can lead to memory leaks when a repository is being cleared. Fix this by moving `reset_commit_grafts()` into "object.c" and making it part of the `struct parsed_object_pool` interface such that we can call it from `parsed_object_pool_clear()`. Adapt `parsed_object_pool_new()` to take and store a reference to its owning repository, which is needed by `unparse_commit()`. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- commit.c | 14 +------------- commit.h | 3 ++- object.c | 14 +++++++++++++- object.h | 4 +++- repository.c | 2 +- shallow.c | 2 +- 6 files changed, 21 insertions(+), 18 deletions(-) diff --git a/commit.c b/commit.c index ec9efc189d..bbef0e81c6 100644 --- a/commit.c +++ b/commit.c @@ -177,7 +177,7 @@ int commit_graft_pos(struct repository *r, const struct object_id *oid) commit_graft_oid_access); } -static void unparse_commit(struct repository *r, const struct object_id *oid) +void unparse_commit(struct repository *r, const struct object_id *oid) { struct commit *c = lookup_commit(r, oid); @@ -318,18 +318,6 @@ int for_each_commit_graft(each_commit_graft_fn fn, void *cb_data) return ret; } -void reset_commit_grafts(struct repository *r) -{ - int i; - - for (i = 0; i < r->parsed_objects->grafts_nr; i++) { - unparse_commit(r, &r->parsed_objects->grafts[i]->oid); - free(r->parsed_objects->grafts[i]); - } - r->parsed_objects->grafts_nr = 0; - r->parsed_objects->commit_graft_prepared = 0; -} - struct commit_buffer { void *buffer; unsigned long size; diff --git a/commit.h b/commit.h index d62b1d93f9..5ba0f77b1e 100644 --- a/commit.h +++ b/commit.h @@ -108,6 +108,8 @@ static inline int repo_parse_commit_no_graph(struct repository *r, void parse_commit_or_die(struct commit *item); +void unparse_commit(struct repository *r, const struct object_id *oid); + struct buffer_slab; struct buffer_slab *allocate_commit_buffer_slab(void); void free_commit_buffer_slab(struct buffer_slab *bs); @@ -240,7 +242,6 @@ int commit_graft_pos(struct repository *r, const struct object_id *oid); int register_commit_graft(struct repository *r, struct commit_graft *, int); void prepare_commit_graft(struct repository *r); struct commit_graft *lookup_commit_graft(struct repository *r, const struct object_id *oid); -void reset_commit_grafts(struct repository *r); struct commit *get_fork_point(const char *refname, struct commit *commit); diff --git a/object.c b/object.c index d756c7f2ea..94ea8fb8d2 100644 --- a/object.c +++ b/object.c @@ -545,11 +545,12 @@ void repo_clear_commit_marks(struct repository *r, unsigned int flags) } } -struct parsed_object_pool *parsed_object_pool_new(void) +struct parsed_object_pool *parsed_object_pool_new(struct repository *repo) { struct parsed_object_pool *o = xmalloc(sizeof(*o)); memset(o, 0, sizeof(*o)); + o->repo = repo; o->blob_state = allocate_alloc_state(); o->tree_state = allocate_alloc_state(); o->commit_state = allocate_alloc_state(); @@ -628,6 +629,16 @@ void raw_object_store_clear(struct raw_object_store *o) hashmap_clear(&o->pack_map); } +void parsed_object_pool_reset_commit_grafts(struct parsed_object_pool *o) +{ + for (int i = 0; i < o->grafts_nr; i++) { + unparse_commit(o->repo, &o->grafts[i]->oid); + free(o->grafts[i]); + } + o->grafts_nr = 0; + o->commit_graft_prepared = 0; +} + void parsed_object_pool_clear(struct parsed_object_pool *o) { /* @@ -659,6 +670,7 @@ void parsed_object_pool_clear(struct parsed_object_pool *o) free_commit_buffer_slab(o->buffer_slab); o->buffer_slab = NULL; + parsed_object_pool_reset_commit_grafts(o); clear_alloc_state(o->blob_state); clear_alloc_state(o->tree_state); clear_alloc_state(o->commit_state); diff --git a/object.h b/object.h index 05691486eb..17f32f1103 100644 --- a/object.h +++ b/object.h @@ -7,6 +7,7 @@ struct buffer_slab; struct repository; struct parsed_object_pool { + struct repository *repo; struct object **obj_hash; int nr_objs, obj_hash_size; @@ -31,8 +32,9 @@ struct parsed_object_pool { struct buffer_slab *buffer_slab; }; -struct parsed_object_pool *parsed_object_pool_new(void); +struct parsed_object_pool *parsed_object_pool_new(struct repository *repo); void parsed_object_pool_clear(struct parsed_object_pool *o); +void parsed_object_pool_reset_commit_grafts(struct parsed_object_pool *o); struct object_list { struct object *item; diff --git a/repository.c b/repository.c index 9825a30899..e6fc2c6aa9 100644 --- a/repository.c +++ b/repository.c @@ -54,7 +54,7 @@ void initialize_repository(struct repository *repo) { repo->objects = raw_object_store_new(); repo->remote_state = remote_state_new(); - repo->parsed_objects = parsed_object_pool_new(); + repo->parsed_objects = parsed_object_pool_new(repo); ALLOC_ARRAY(repo->index, 1); index_state_init(repo->index, repo); diff --git a/shallow.c b/shallow.c index b8cd051e3b..a10cf9e9d5 100644 --- a/shallow.c +++ b/shallow.c @@ -97,7 +97,7 @@ static void reset_repository_shallow(struct repository *r) { r->parsed_objects->is_shallow = -1; stat_validity_clear(r->parsed_objects->shallow_stat); - reset_commit_grafts(r); + parsed_object_pool_reset_commit_grafts(r->parsed_objects); } int commit_shallow_file(struct repository *r, struct shallow_lock *lk) From 14c0ea0f6f7a0898214d8fa822a2392ef3a00f53 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 5 Sep 2024 12:09:15 +0200 Subject: [PATCH 13/22] shallow: free grafts when unregistering them When removing a graft via `unregister_shallow()` we remove it from the grafts array, but do not free the structure. Fix this to plug the leak. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- shallow.c | 4 +++- t/t5537-fetch-shallow.sh | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/shallow.c b/shallow.c index a10cf9e9d5..7e0ee96ead 100644 --- a/shallow.c +++ b/shallow.c @@ -51,10 +51,12 @@ int unregister_shallow(const struct object_id *oid) int pos = commit_graft_pos(the_repository, oid); if (pos < 0) return -1; - if (pos + 1 < the_repository->parsed_objects->grafts_nr) + if (pos + 1 < the_repository->parsed_objects->grafts_nr) { + free(the_repository->parsed_objects->grafts[pos]); MOVE_ARRAY(the_repository->parsed_objects->grafts + pos, the_repository->parsed_objects->grafts + pos + 1, the_repository->parsed_objects->grafts_nr - pos - 1); + } the_repository->parsed_objects->grafts_nr--; return 0; } diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh index 37f7547a4c..cae4d400f3 100755 --- a/t/t5537-fetch-shallow.sh +++ b/t/t5537-fetch-shallow.sh @@ -5,6 +5,7 @@ test_description='fetch/clone from a shallow clone' GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh commit() { From 16c6fb5a94231e76f618eefc7a683fd12091968a Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 5 Sep 2024 12:09:17 +0200 Subject: [PATCH 14/22] shallow: fix leaking members of `struct shallow_info` We do not free several struct members in `clear_shallow_info()`. Fix this to plug the resulting leaks. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- shallow.c | 9 +++++++++ t/t5538-push-shallow.sh | 1 + 2 files changed, 10 insertions(+) diff --git a/shallow.c b/shallow.c index 7e0ee96ead..dcebc263d7 100644 --- a/shallow.c +++ b/shallow.c @@ -489,6 +489,15 @@ void prepare_shallow_info(struct shallow_info *info, struct oid_array *sa) void clear_shallow_info(struct shallow_info *info) { + if (info->used_shallow) { + for (size_t i = 0; i < info->shallow->nr; i++) + free(info->used_shallow[i]); + free(info->used_shallow); + } + + free(info->need_reachability_test); + free(info->reachable); + free(info->shallow_ref); free(info->ours); free(info->theirs); } diff --git a/t/t5538-push-shallow.sh b/t/t5538-push-shallow.sh index e91fcc173e..6adc3a20a4 100755 --- a/t/t5538-push-shallow.sh +++ b/t/t5538-push-shallow.sh @@ -5,6 +5,7 @@ test_description='push from/to a shallow clone' GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh commit() { From a46f231975f4c7ac94af0847f4b3bb8b11493d80 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 5 Sep 2024 12:09:20 +0200 Subject: [PATCH 15/22] negotiator/skipping: fix leaking commit entries When releasing the skipping negotiator we free its priority queue, but not the contained entries. Fix this to plug a memory leak. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- negotiator/skipping.c | 7 +++++-- t/t5552-skipping-fetch-negotiator.sh | 2 ++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/negotiator/skipping.c b/negotiator/skipping.c index f65d47858b..b738fe4fae 100644 --- a/negotiator/skipping.c +++ b/negotiator/skipping.c @@ -247,8 +247,11 @@ static int ack(struct fetch_negotiator *n, struct commit *c) static void release(struct fetch_negotiator *n) { - clear_prio_queue(&((struct data *)n->data)->rev_list); - FREE_AND_NULL(n->data); + struct data *data = n->data; + for (int i = 0; i < data->rev_list.nr; i++) + free(data->rev_list.array[i].data); + clear_prio_queue(&data->rev_list); + FREE_AND_NULL(data); } void skipping_negotiator_init(struct fetch_negotiator *negotiator) diff --git a/t/t5552-skipping-fetch-negotiator.sh b/t/t5552-skipping-fetch-negotiator.sh index b55a9f65e6..4f2e5ae8df 100755 --- a/t/t5552-skipping-fetch-negotiator.sh +++ b/t/t5552-skipping-fetch-negotiator.sh @@ -1,6 +1,8 @@ #!/bin/sh test_description='test skipping fetch negotiator' + +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh test_expect_success 'fetch.negotiationalgorithm config' ' From 860b6780163ade3bb705d6565619ec13efcc77c6 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 5 Sep 2024 12:09:23 +0200 Subject: [PATCH 16/22] builtin/repack: fix leaking line buffer when packing promisors In `repack_promisor_objects()` we read output from git-pack-objects(1) line by line, using `strbuf_getline_lf()`. We never free the line buffer, causing a memory leak. Plug it. This leak is being hit in t5616, but plugging it alone is not sufficient to make the whole test suite leak free. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/repack.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/builtin/repack.c b/builtin/repack.c index 62cfa50c50..2b9bf0318a 100644 --- a/builtin/repack.c +++ b/builtin/repack.c @@ -425,9 +425,11 @@ static void repack_promisor_objects(const struct pack_objects_args *args, free(promisor_name); } + fclose(out); if (finish_command(&cmd)) die(_("could not finish pack-objects to repack promisor objects")); + strbuf_release(&line); } struct pack_geometry { From 149c83e0aa1ee75b2da9c27ce5a819025260b4da Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 5 Sep 2024 12:09:26 +0200 Subject: [PATCH 17/22] builtin/pack-objects: plug leaking list of keep-packs The `--keep-pack` option of git-pack-objects(1) populates the arguments into a string list. And while the list is marked as `NODUP` and thus won't duplicate the strings, the list entries themselves still need to be free'd. We don't though, causing a leak. Plug it. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/pack-objects.c | 1 + t/t5616-partial-clone.sh | 1 + 2 files changed, 2 insertions(+) diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index c481feadbf..ab78d72e27 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -4641,6 +4641,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix) cleanup: clear_packing_data(&to_pack); list_objects_filter_release(&filter_options); + string_list_clear(&keep_pack_list, 0); strvec_clear(&rp); return 0; diff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh index 2da7291e37..467c46dccf 100755 --- a/t/t5616-partial-clone.sh +++ b/t/t5616-partial-clone.sh @@ -5,6 +5,7 @@ test_description='git partial clone' GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh # create a normal "src" repo where we can later create new commits. From ee087c29c89ad54980f2521641f5f182f6adbc79 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 5 Sep 2024 12:09:31 +0200 Subject: [PATCH 18/22] builtin/grep: fix leaking object context Even when `get_oid_with_context()` fails it may have allocated some data in the object context. But we do not release it in git-grep(1) when the call fails, leading to a memory leak. Plug it. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/grep.c | 1 + t/t6132-pathspec-exclude.sh | 1 + t/t6135-pathspec-with-attrs.sh | 2 ++ 3 files changed, 4 insertions(+) diff --git a/builtin/grep.c b/builtin/grep.c index dfc3c3e8bd..dda4582d64 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -1133,6 +1133,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix) &oid, &oc)) { if (seen_dashdash) die(_("unable to resolve revision: %s"), arg); + object_context_release(&oc); break; } diff --git a/t/t6132-pathspec-exclude.sh b/t/t6132-pathspec-exclude.sh index 9fdafeb1e9..f31c09c056 100755 --- a/t/t6132-pathspec-exclude.sh +++ b/t/t6132-pathspec-exclude.sh @@ -2,6 +2,7 @@ test_description='test case exclude pathspec' +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh test_expect_success 'setup' ' diff --git a/t/t6135-pathspec-with-attrs.sh b/t/t6135-pathspec-with-attrs.sh index 120dcd74a5..794bc7daf0 100755 --- a/t/t6135-pathspec-with-attrs.sh +++ b/t/t6135-pathspec-with-attrs.sh @@ -1,6 +1,8 @@ #!/bin/sh test_description='test labels in pathspecs' + +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh test_expect_success 'setup a tree' ' From 68bd0a94bedf520bc67ca38216e839770f291296 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 5 Sep 2024 12:09:34 +0200 Subject: [PATCH 19/22] builtin/fmt-merge-msg: fix leaking buffers Fix leaking input and output buffers in git-fmt-merge-msg(1). Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/fmt-merge-msg.c | 2 ++ t/t6200-fmt-merge-msg.sh | 1 + 2 files changed, 3 insertions(+) diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c index 957786d1b3..0b162f8fab 100644 --- a/builtin/fmt-merge-msg.c +++ b/builtin/fmt-merge-msg.c @@ -67,6 +67,8 @@ int cmd_fmt_merge_msg(int argc, const char **argv, const char *prefix) return ret; write_in_full(STDOUT_FILENO, output.buf, output.len); + strbuf_release(&input); + strbuf_release(&output); free(inpath); return 0; } diff --git a/t/t6200-fmt-merge-msg.sh b/t/t6200-fmt-merge-msg.sh index 5a221f8ef1..ac57b0e4ae 100755 --- a/t/t6200-fmt-merge-msg.sh +++ b/t/t6200-fmt-merge-msg.sh @@ -8,6 +8,7 @@ test_description='fmt-merge-msg test' 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-gpg.sh" From 2a0147089151e3dd2cc8e6c91ecaf45fb416630e Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 5 Sep 2024 12:09:36 +0200 Subject: [PATCH 20/22] match-trees: fix leaking prefixes in `shift_tree()` In `shift_tree()` we allocate two empty strings that we end up passing to `match_trees()`. If that function finds a better match it will update these pointers to point to a newly allocated strings, freeing the old strings. We never free the final results though, neither the ones we have allocated ourselves, nor the one that `match_trees()` might've returned to us. Fix the resulting memory leaks by creating a common exit path where we free them. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- match-trees.c | 10 +++++++--- t/t6409-merge-subtree.sh | 1 + 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/match-trees.c b/match-trees.c index f17c74d483..147b03abf1 100644 --- a/match-trees.c +++ b/match-trees.c @@ -294,18 +294,22 @@ void shift_tree(struct repository *r, unsigned short mode; if (!*del_prefix) - return; + goto out; if (get_tree_entry(r, hash2, del_prefix, shifted, &mode)) die("cannot find path %s in tree %s", del_prefix, oid_to_hex(hash2)); - return; + goto out; } if (!*add_prefix) - return; + goto out; splice_tree(hash1, add_prefix, hash2, shifted); + +out: + free(add_prefix); + free(del_prefix); } /* diff --git a/t/t6409-merge-subtree.sh b/t/t6409-merge-subtree.sh index e9ba6f1690..528615b981 100755 --- a/t/t6409-merge-subtree.sh +++ b/t/t6409-merge-subtree.sh @@ -5,6 +5,7 @@ test_description='subtree merge strategy' 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 ' From ed78f048ae369e9076689cca71ff99f2baa90cd2 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 5 Sep 2024 12:09:39 +0200 Subject: [PATCH 21/22] merge-ort: fix two leaks when handling directory rename modifications There are two leaks in `apply_directory_rename_modifications()`: - We do not release the `dirs_to_insert` string list. - We do not release some `conflict_info` we put into the `opt->priv->paths` string map. The former is trivial to fix. The latter is a bit less straight forward: the `util` pointer of the string map may sometimes point to data that has been allocated via `CALLOC()`, while at other times it may point to data that has been allocated via a `mem_pool`. It very much seems like an oversight that we didn't also allocate the conflict info in this code path via the memory pool, though. So let's fix that, which will also plug the memory leak for us. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- merge-ort.c | 4 +++- t/t6423-merge-rename-directories.sh | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/merge-ort.c b/merge-ort.c index 3752c7e595..0ed3cd06b1 100644 --- a/merge-ort.c +++ b/merge-ort.c @@ -2710,7 +2710,7 @@ static void apply_directory_rename_modifications(struct merge_options *opt, struct conflict_info *dir_ci; char *cur_dir = dirs_to_insert.items[i].string; - CALLOC_ARRAY(dir_ci, 1); + dir_ci = mem_pool_calloc(&opt->priv->pool, 1, sizeof(*dir_ci)); dir_ci->merged.directory_name = parent_name; len = strlen(parent_name); @@ -2838,6 +2838,8 @@ static void apply_directory_rename_modifications(struct merge_options *opt, * Finally, record the new location. */ pair->two->path = new_path; + + string_list_clear(&dirs_to_insert, 0); } /*** Function Grouping: functions related to regular rename detection ***/ diff --git a/t/t6423-merge-rename-directories.sh b/t/t6423-merge-rename-directories.sh index 88d1cf2cde..4aaaf38be6 100755 --- a/t/t6423-merge-rename-directories.sh +++ b/t/t6423-merge-rename-directories.sh @@ -25,6 +25,7 @@ test_description="recursive merge with directory renames" # underscore notation is to differentiate different # files that might be renamed into each other's paths.) +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh . "$TEST_DIRECTORY"/lib-merge.sh From 46f6ca2a68e02dd68132ed0b64cd55a8b6569e29 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 5 Sep 2024 12:09:41 +0200 Subject: [PATCH 22/22] builtin/repack: fix leaking keep-pack list The list of packs to keep is populated via a command line option but never free'd. Plug this memory leak. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/repack.c | 1 + t/t6500-gc.sh | 1 + t/t7703-repack-geometric.sh | 1 + 3 files changed, 3 insertions(+) diff --git a/builtin/repack.c b/builtin/repack.c index 2b9bf0318a..367e970dcf 100644 --- a/builtin/repack.c +++ b/builtin/repack.c @@ -1525,6 +1525,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix) } cleanup: + string_list_clear(&keep_pack_list, 0); string_list_clear(&names, 1); existing_packs_release(&existing); free_pack_geometry(&geometry); diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh index 1b5909d1b7..58654b3437 100755 --- a/t/t6500-gc.sh +++ b/t/t6500-gc.sh @@ -3,6 +3,7 @@ test_description='basic git gc tests ' +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh . "$TEST_DIRECTORY"/lib-terminal.sh diff --git a/t/t7703-repack-geometric.sh b/t/t7703-repack-geometric.sh index 9fc1626fbf..8877aea98b 100755 --- a/t/t7703-repack-geometric.sh +++ b/t/t7703-repack-geometric.sh @@ -2,6 +2,7 @@ test_description='git repack --geometric works correctly' +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh GIT_TEST_MULTI_PACK_INDEX=0