From 99b6c45d8f846ed055ba814d985084733d36dabc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Fri, 1 Jul 2022 12:42:50 +0200 Subject: [PATCH 01/11] check-ref-format: fix trivial memory leak MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix a memory leak in "git check-ref-format" that's been present in the code in one form or another since 38eedc634bc (git check-ref-format --print, 2009-10-12), the code got substantially refactored in cfbe22f03f9 (check-ref-format: handle subcommands in separate functions, 2010-08-05). As a result we can mark a test as passing with SANITIZE=leak using "TEST_PASSES_SANITIZE_LEAK=true". Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- builtin/check-ref-format.c | 11 ++++++++--- t/t1402-check-ref-format.sh | 1 + 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c index bc67d3f0a8..fd0e5f8683 100644 --- a/builtin/check-ref-format.c +++ b/builtin/check-ref-format.c @@ -57,6 +57,8 @@ int cmd_check_ref_format(int argc, const char **argv, const char *prefix) int normalize = 0; int flags = 0; const char *refname; + char *to_free = NULL; + int ret = 1; if (argc == 2 && !strcmp(argv[1], "-h")) usage(builtin_check_ref_format_usage); @@ -81,11 +83,14 @@ int cmd_check_ref_format(int argc, const char **argv, const char *prefix) refname = argv[i]; if (normalize) - refname = collapse_slashes(refname); + refname = to_free = collapse_slashes(refname); if (check_refname_format(refname, flags)) - return 1; + goto cleanup; if (normalize) printf("%s\n", refname); - return 0; + ret = 0; +cleanup: + free(to_free); + return ret; } diff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh index cabc516ae9..5ed9d7318e 100755 --- a/t/t1402-check-ref-format.sh +++ b/t/t1402-check-ref-format.sh @@ -2,6 +2,7 @@ test_description='Test git check-ref-format' +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh valid_ref() { From 74a06a9f2103332d50ddc17dcd6a0a153a5e377d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Fri, 1 Jul 2022 12:42:51 +0200 Subject: [PATCH 02/11] clone: fix memory leak in wanted_peer_refs() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix a memory leak added in 0ec4b1650cc (clone: fix ref selection in --single-branch --branch=xxx, 2012-06-22). Whether we get our "remote_head" from copy_ref() directly, or with a call to guess_remote_head() it'll be the result of a copy_ref() in either case, as guess_remote_head() is a wrapper for copy_ref() (or it returns NULL). We can't mark any tests passing passing with SANITIZE=leak using "TEST_PASSES_SANITIZE_LEAK=true" as a result of this change, but e.g. "t/t1500-rev-parse.sh" now gets closer to passing. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- builtin/clone.c | 1 + 1 file changed, 1 insertion(+) diff --git a/builtin/clone.c b/builtin/clone.c index 89a91b0017..c43c85dad0 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -494,6 +494,7 @@ static struct ref *wanted_peer_refs(const struct ref *refs, /* if --branch=tag, pull the requested tag explicitly */ get_fetch_map(remote_head, tag_refspec, &tail, 0); } + free_refs(remote_head); } else { int i; for (i = 0; i < refspec->nr; i++) From bc57ba1d54f3133cd46481532d97a8346b62d8aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Fri, 1 Jul 2022 12:42:52 +0200 Subject: [PATCH 03/11] submodule.c: free() memory from xgetcwd() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix a memory leak in code added in bf0231c6614 (rev-parse: add --show-superproject-working-tree, 2017-03-08), we should never have made the result of xgetcwd() a "const char *", as we return a strbuf_detach()'d value. Let's fix that and free() it when we're done with it. We can't mark any tests passing passing with SANITIZE=leak using "TEST_PASSES_SANITIZE_LEAK=true" as a result of this change, but e.g. "t/t1500-rev-parse.sh" now gets closer to passing. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- submodule.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/submodule.c b/submodule.c index 4e299f578f..06073b2e7b 100644 --- a/submodule.c +++ b/submodule.c @@ -2388,7 +2388,7 @@ int get_superproject_working_tree(struct strbuf *buf) struct child_process cp = CHILD_PROCESS_INIT; struct strbuf sb = STRBUF_INIT; struct strbuf one_up = STRBUF_INIT; - const char *cwd = xgetcwd(); + char *cwd = xgetcwd(); int ret = 0; const char *subpath; int code; @@ -2451,6 +2451,7 @@ int get_superproject_working_tree(struct strbuf *buf) ret = 1; free(super_wt); } + free(cwd); strbuf_release(&sb); code = finish_command(&cp); From fd74ac95ac31cefee41cb732255c25d910008fff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Fri, 1 Jul 2022 12:42:53 +0200 Subject: [PATCH 04/11] revert: free "struct replay_opts" members MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Call the release_revisions() function added in 1878b5edc03 (revision.[ch]: provide and start using a release_revisions(), 2022-04-13) in cmd_revert(), as well as freeing the xmalloc()'d "revs" member itself. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- builtin/revert.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/builtin/revert.c b/builtin/revert.c index f84c253f4c..2554f9099c 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -246,6 +246,9 @@ 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); return res; } From d90dafbe3180aa0ad095a5f2fa9c83bfa3d3767d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Fri, 1 Jul 2022 12:42:54 +0200 Subject: [PATCH 05/11] cat-file: fix a memory leak in --batch-command mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix a memory leak introduced in 440c705ea63 (cat-file: add --batch-command mode, 2022-02-18). The free_cmds() function was only called on "queued_nr" if we had a "flush" command. As the "without flush for blob info" test added in the same commit shows we can't rely on that, so let's call free_cmds() again at the end. Since "nr" follows the usual pattern of being set to 0 if we've free()'d the memory already it's OK to call it twice, even in cases where we are doing a "flush". Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- builtin/cat-file.c | 1 + 1 file changed, 1 insertion(+) diff --git a/builtin/cat-file.c b/builtin/cat-file.c index 50cf38999d..ac0459f96b 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -655,6 +655,7 @@ static void batch_objects_command(struct batch_options *opt, free_cmds(queued_cmd, &nr); } + free_cmds(queued_cmd, &nr); free(queued_cmd); strbuf_release(&input); } From 480a0e30a78a6a86e1ea97059f72e4bf4299d052 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Fri, 1 Jul 2022 12:42:55 +0200 Subject: [PATCH 06/11] merge-file: refactor for subsequent memory leak fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refactor the code in builtin/merge-file.c to: * Use the initializer to zero out "mmfs", and use modern C syntax for the rest. * Refactor the the inner loop to use a variable and "if/else if" pattern followed by "return". This will make a change to change it to a "goto cleanup" pattern smaller. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- builtin/merge-file.c | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/builtin/merge-file.c b/builtin/merge-file.c index e695867ee5..793817f3cb 100644 --- a/builtin/merge-file.c +++ b/builtin/merge-file.c @@ -25,10 +25,10 @@ static int label_cb(const struct option *opt, const char *arg, int unset) int cmd_merge_file(int argc, const char **argv, const char *prefix) { - const char *names[3] = { NULL, NULL, NULL }; - mmfile_t mmfs[3]; - mmbuffer_t result = {NULL, 0}; - xmparam_t xmp = {{0}}; + const char *names[3] = { 0 }; + mmfile_t mmfs[3] = { 0 }; + mmbuffer_t result = { 0 }; + xmparam_t xmp = { 0 }; int ret = 0, i = 0, to_stdout = 0; int quiet = 0; struct option options[] = { @@ -71,21 +71,23 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix) for (i = 0; i < 3; i++) { char *fname; - int ret; + mmfile_t *mmf = mmfs + i; if (!names[i]) names[i] = argv[i]; fname = prefix_filename(prefix, argv[i]); - ret = read_mmfile(mmfs + i, fname); + + if (read_mmfile(mmf, fname)) + ret = -1; + else if (mmf->size > MAX_XDIFF_SIZE || + buffer_is_binary(mmf->ptr, mmf->size)) + ret = error("Cannot merge binary files: %s", + argv[i]); + free(fname); if (ret) - return -1; - - if (mmfs[i].size > MAX_XDIFF_SIZE || - buffer_is_binary(mmfs[i].ptr, mmfs[i].size)) - return error("Cannot merge binary files: %s", - argv[i]); + return ret; } xmp.ancestor = names[1]; From e72e12cc02d80e4ebf01e0a3d170617211ae7c70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Fri, 1 Jul 2022 12:42:56 +0200 Subject: [PATCH 07/11] merge-file: fix memory leaks on error path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix a memory leak in "merge-file", we need to loop over the "mmfs" array and free() what we've got so far when we error out. As a result we can mark a test as passing with SANITIZE=leak using "TEST_PASSES_SANITIZE_LEAK=true". Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- builtin/merge-file.c | 10 ++++++---- t/t6403-merge-file.sh | 2 ++ 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/builtin/merge-file.c b/builtin/merge-file.c index 793817f3cb..c923bbf2ab 100644 --- a/builtin/merge-file.c +++ b/builtin/merge-file.c @@ -87,7 +87,8 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix) free(fname); if (ret) - return ret; + goto cleanup; + } xmp.ancestor = names[1]; @@ -95,9 +96,6 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix) xmp.file2 = names[2]; ret = xdl_merge(mmfs + 1, mmfs + 0, mmfs + 2, &xmp, &result); - for (i = 0; i < 3; i++) - free(mmfs[i].ptr); - if (ret >= 0) { const char *filename = argv[0]; char *fpath = prefix_filename(prefix, argv[0]); @@ -118,5 +116,9 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix) if (ret > 127) ret = 127; +cleanup: + for (i = 0; i < 3; i++) + free(mmfs[i].ptr); + return ret; } diff --git a/t/t6403-merge-file.sh b/t/t6403-merge-file.sh index 2f421d967a..1a7082323d 100755 --- a/t/t6403-merge-file.sh +++ b/t/t6403-merge-file.sh @@ -1,6 +1,8 @@ #!/bin/sh test_description='RCS merge replacement: merge-file' + +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh test_expect_success 'setup' ' From 33d0dda633f3748e1af517135a72141f5df35f2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Fri, 1 Jul 2022 12:42:57 +0200 Subject: [PATCH 08/11] checkout: avoid "struct unpack_trees_options" leak MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In 1c41d2805e4 (unpack_trees_options: free messages when done, 2018-05-21) we started calling clear_unpack_trees_porcelain() on this codepath, but missed this error path. We could call clear_unpack_trees_porcelain() just before we error() and return when unmerged_cache() fails, but the more correct fix is to not have the unmerged_cache() check happen in the middle of our "topts" setup. Before 23cbf11b5c0 (merge-recursive: porcelain messages for checkout, 2010-08-11) we would not malloc() to setup our "topts", which is when this started to leak on the error path. Before that this code wasn't conflating the setup of "topts" and the unmerged_cache() call in any meaningful way. The initial version in 782c2d65c24 (Build in checkout, 2008-02-07) just does a "memset" of it, and initializes a single struct member. Then in 8ccba008ee3 (unpack-trees: allow Porcelain to give different error messages, 2008-05-17) we added the initialization of the error message, which as noted above finally started leaking in 23cbf11b5c0. Let's fix the memory leak, and avoid future issues by initializing the "topts" with a helper function. There are no functional changes here. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- builtin/checkout.c | 36 ++++++++++++++++++++++-------------- 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index 2eefda81d8..1109f1301f 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -710,6 +710,26 @@ static void setup_branch_path(struct branch_info *branch) branch->path = strbuf_detach(&buf, NULL); } +static void init_topts(struct unpack_trees_options *topts, int merge, + int show_progress, int overwrite_ignore, + struct commit *old_commit) +{ + memset(topts, 0, sizeof(*topts)); + topts->head_idx = -1; + topts->src_index = &the_index; + topts->dst_index = &the_index; + + setup_unpack_trees_porcelain(topts, "checkout"); + + topts->initial_checkout = is_cache_unborn(); + topts->update = 1; + topts->merge = 1; + topts->quiet = merge && old_commit; + topts->verbose_update = show_progress; + topts->fn = twoway_merge; + topts->preserve_ignored = !overwrite_ignore; +} + static int merge_working_tree(const struct checkout_opts *opts, struct branch_info *old_branch_info, struct branch_info *new_branch_info, @@ -740,13 +760,6 @@ static int merge_working_tree(const struct checkout_opts *opts, struct unpack_trees_options topts; const struct object_id *old_commit_oid; - memset(&topts, 0, sizeof(topts)); - topts.head_idx = -1; - topts.src_index = &the_index; - topts.dst_index = &the_index; - - setup_unpack_trees_porcelain(&topts, "checkout"); - refresh_cache(REFRESH_QUIET); if (unmerged_cache()) { @@ -755,17 +768,12 @@ static int merge_working_tree(const struct checkout_opts *opts, } /* 2-way merge to the new branch */ - topts.initial_checkout = is_cache_unborn(); - topts.update = 1; - topts.merge = 1; - topts.quiet = opts->merge && old_branch_info->commit; - topts.verbose_update = opts->show_progress; - topts.fn = twoway_merge; + init_topts(&topts, opts->merge, opts->show_progress, + opts->overwrite_ignore, old_branch_info->commit); init_checkout_metadata(&topts.meta, new_branch_info->refname, new_branch_info->commit ? &new_branch_info->commit->object.oid : &new_branch_info->oid, NULL); - topts.preserve_ignored = !opts->overwrite_ignore; old_commit_oid = old_branch_info->commit ? &old_branch_info->commit->object.oid : From 55916bba0f4805a3bd0c1891c48effbfe1d12536 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Fri, 1 Jul 2022 12:42:58 +0200 Subject: [PATCH 09/11] gc: fix a memory leak MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix a memory leak in code added in 41abfe15d95 (maintenance: add pack-refs task, 2021-02-09), we need to call strvec_clear() on the "struct strvec" that we initialized. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- builtin/gc.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/builtin/gc.c b/builtin/gc.c index 021e9256ae..eeff2b760e 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -168,9 +168,15 @@ struct maintenance_run_opts; static int maintenance_task_pack_refs(MAYBE_UNUSED struct maintenance_run_opts *opts) { struct strvec pack_refs_cmd = STRVEC_INIT; + int ret; + strvec_pushl(&pack_refs_cmd, "pack-refs", "--all", "--prune", NULL); - return run_command_v_opt(pack_refs_cmd.v, RUN_GIT_CMD); + ret = run_command_v_opt(pack_refs_cmd.v, RUN_GIT_CMD); + + strvec_clear(&pack_refs_cmd); + + return ret; } static int too_many_loose_objects(void) From 27472b5195e3e8e888be0fdc3a7a22687cd808fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Fri, 1 Jul 2022 12:42:59 +0200 Subject: [PATCH 10/11] cat-file: fix a common "struct object_context" memory leak MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix a memory leak where "cat-file" will leak the "path" member. See e5fba602e59 (textconv: support for cat_file, 2010-06-15) for the code that introduced the offending get_oid_with_context() call (called get_sha1_with_context() at the time). As a result we can mark several tests as passing with SANITIZE=leak using "TEST_PASSES_SANITIZE_LEAK=true". As noted in dc944b65f1d (get_sha1_with_context: dynamically allocate oc->path, 2017-05-19) callers must free the "path" member. That same commit added the relevant free() to this function, but we weren't catching cases where we'd return early. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- builtin/cat-file.c | 32 +++++++++++++++++++--------- t/t0028-working-tree-encoding.sh | 1 + t/t1051-large-conversion.sh | 2 ++ t/t3304-notes-mixed.sh | 1 + t/t4044-diff-index-unique-abbrev.sh | 2 ++ t/t4140-apply-ita.sh | 1 + t/t5314-pack-cycle-detection.sh | 4 ++-- t/t6422-merge-rename-corner-cases.sh | 1 + t/t8007-cat-file-textconv.sh | 2 ++ t/t8010-cat-file-filters.sh | 2 ++ t/t9104-git-svn-follow-parent.sh | 1 - t/t9132-git-svn-broken-symlink.sh | 1 - t/t9301-fast-import-notes.sh | 1 + 13 files changed, 37 insertions(+), 14 deletions(-) diff --git a/builtin/cat-file.c b/builtin/cat-file.c index ac0459f96b..f42782e955 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -71,6 +71,7 @@ static int stream_blob(const struct object_id *oid) static int cat_one_file(int opt, const char *exp_type, const char *obj_name, int unknown_type) { + int ret; struct object_id oid; enum object_type type; char *buf; @@ -106,7 +107,8 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name, if (sb.len) { printf("%s\n", sb.buf); strbuf_release(&sb); - return 0; + ret = 0; + goto cleanup; } break; @@ -115,7 +117,8 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name, if (oid_object_info_extended(the_repository, &oid, &oi, flags) < 0) die("git cat-file: could not get object info"); printf("%"PRIuMAX"\n", (uintmax_t)size); - return 0; + ret = 0; + goto cleanup; case 'e': return !has_object_file(&oid); @@ -123,8 +126,10 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name, case 'w': if (filter_object(path, obj_context.mode, - &oid, &buf, &size)) - return -1; + &oid, &buf, &size)) { + ret = -1; + goto cleanup; + } break; case 'c': @@ -143,11 +148,14 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name, const char *ls_args[3] = { NULL }; ls_args[0] = "ls-tree"; ls_args[1] = obj_name; - return cmd_ls_tree(2, ls_args, NULL); + ret = cmd_ls_tree(2, ls_args, NULL); + goto cleanup; } - if (type == OBJ_BLOB) - return stream_blob(&oid); + if (type == OBJ_BLOB) { + ret = stream_blob(&oid); + goto cleanup; + } buf = read_object_file(&oid, &type, &size); if (!buf) die("Cannot read object %s", obj_name); @@ -172,8 +180,10 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name, } else oidcpy(&blob_oid, &oid); - if (oid_object_info(the_repository, &blob_oid, NULL) == OBJ_BLOB) - return stream_blob(&blob_oid); + if (oid_object_info(the_repository, &blob_oid, NULL) == OBJ_BLOB) { + ret = stream_blob(&blob_oid); + goto cleanup; + } /* * we attempted to dereference a tag to a blob * and failed; there may be new dereference @@ -193,9 +203,11 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name, die("git cat-file %s: bad file", obj_name); write_or_die(1, buf, size); + ret = 0; +cleanup: free(buf); free(obj_context.path); - return 0; + return ret; } struct expand_data { diff --git a/t/t0028-working-tree-encoding.sh b/t/t0028-working-tree-encoding.sh index 82905a2156..416eeabdb9 100755 --- a/t/t0028-working-tree-encoding.sh +++ b/t/t0028-working-tree-encoding.sh @@ -5,6 +5,7 @@ test_description='working-tree-encoding conversion via gitattributes' 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-encoding.sh" diff --git a/t/t1051-large-conversion.sh b/t/t1051-large-conversion.sh index 042b0e4429..f6709c9f56 100755 --- a/t/t1051-large-conversion.sh +++ b/t/t1051-large-conversion.sh @@ -1,6 +1,8 @@ #!/bin/sh test_description='test conversion filters on large files' + +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh set_attr() { diff --git a/t/t3304-notes-mixed.sh b/t/t3304-notes-mixed.sh index 03dfcd3954..2c3a245266 100755 --- a/t/t3304-notes-mixed.sh +++ b/t/t3304-notes-mixed.sh @@ -5,6 +5,7 @@ test_description='Test notes trees that also contain non-notes' GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh number_of_commits=100 diff --git a/t/t4044-diff-index-unique-abbrev.sh b/t/t4044-diff-index-unique-abbrev.sh index 4701796d10..29e49d2290 100755 --- a/t/t4044-diff-index-unique-abbrev.sh +++ b/t/t4044-diff-index-unique-abbrev.sh @@ -1,6 +1,8 @@ #!/bin/sh test_description='test unique sha1 abbreviation on "index from..to" line' + +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh test_expect_success 'setup' ' diff --git a/t/t4140-apply-ita.sh b/t/t4140-apply-ita.sh index c614eaf04c..b375aca0d7 100755 --- a/t/t4140-apply-ita.sh +++ b/t/t4140-apply-ita.sh @@ -2,6 +2,7 @@ test_description='git apply of i-t-a file' +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh test_expect_success setup ' diff --git a/t/t5314-pack-cycle-detection.sh b/t/t5314-pack-cycle-detection.sh index 0aec8619e2..73a241743a 100755 --- a/t/t5314-pack-cycle-detection.sh +++ b/t/t5314-pack-cycle-detection.sh @@ -49,10 +49,10 @@ Then no matter which order we start looking at the packs in, we know that we will always find a delta for "file", because its lookup will always come immediately after the lookup for "dummy". ' + +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh - - # Create a pack containing the tree $1 and blob $1:file, with # the latter stored as a delta against $2:file. # diff --git a/t/t6422-merge-rename-corner-cases.sh b/t/t6422-merge-rename-corner-cases.sh index bf4ce3c63d..9b65768aed 100755 --- a/t/t6422-merge-rename-corner-cases.sh +++ b/t/t6422-merge-rename-corner-cases.sh @@ -6,6 +6,7 @@ test_description="recursive merge corner cases w/ renames but not criss-crosses" 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-merge.sh diff --git a/t/t8007-cat-file-textconv.sh b/t/t8007-cat-file-textconv.sh index b067983ba1..c8266f17f1 100755 --- a/t/t8007-cat-file-textconv.sh +++ b/t/t8007-cat-file-textconv.sh @@ -1,6 +1,8 @@ #!/bin/sh test_description='git cat-file textconv support' + +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh cat >helper <<'EOF' diff --git a/t/t8010-cat-file-filters.sh b/t/t8010-cat-file-filters.sh index 31de4b64dc..ca04242ca0 100755 --- a/t/t8010-cat-file-filters.sh +++ b/t/t8010-cat-file-filters.sh @@ -1,6 +1,8 @@ #!/bin/sh test_description='git cat-file filters support' + +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh test_expect_success 'setup ' ' diff --git a/t/t9104-git-svn-follow-parent.sh b/t/t9104-git-svn-follow-parent.sh index 5cf2ef4b8b..85d735861f 100755 --- a/t/t9104-git-svn-follow-parent.sh +++ b/t/t9104-git-svn-follow-parent.sh @@ -5,7 +5,6 @@ test_description='git svn fetching' -TEST_FAILS_SANITIZE_LEAK=true . ./lib-git-svn.sh test_expect_success 'initialize repo' ' diff --git a/t/t9132-git-svn-broken-symlink.sh b/t/t9132-git-svn-broken-symlink.sh index 4d8d0584b7..aeceffaf7b 100755 --- a/t/t9132-git-svn-broken-symlink.sh +++ b/t/t9132-git-svn-broken-symlink.sh @@ -2,7 +2,6 @@ test_description='test that git handles an svn repository with empty symlinks' -TEST_FAILS_SANITIZE_LEAK=true . ./lib-git-svn.sh test_expect_success 'load svn dumpfile' ' svnadmin load "$rawsvnrepo" < Date: Fri, 1 Jul 2022 12:43:00 +0200 Subject: [PATCH 11/11] pull: fix a "struct oid_array" memory leak MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix a memory leak introduced in 44c175c7a46 (pull: error on no merge candidates, 2015-06-18). As a result we can mark several tests as passing with SANITIZE=leak using "TEST_PASSES_SANITIZE_LEAK=true". Removing the "int ret = 0" assignment added here in a6d7eb2c7a6 (pull: optionally rebase submodules (remote submodule changes only), 2017-06-23) is not a logic error, it could always have been left uninitialized (as "int ret"), now that we'll use the "ret" from the upper scope we can drop the assignment in the "opt_rebase" branch. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- builtin/pull.c | 16 ++++++++++------ t/t5524-pull-msg.sh | 1 + t/t6417-merge-ours-theirs.sh | 1 + t/t9101-git-svn-props.sh | 1 - 4 files changed, 12 insertions(+), 7 deletions(-) diff --git a/builtin/pull.c b/builtin/pull.c index 01155ba67b..403a24d7ca 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -990,6 +990,7 @@ int cmd_pull(int argc, const char **argv, const char *prefix) int rebase_unspecified = 0; int can_ff; int divergent; + int ret; if (!getenv("GIT_REFLOG_ACTION")) set_reflog_message(argc, argv); @@ -1100,7 +1101,8 @@ int cmd_pull(int argc, const char **argv, const char *prefix) if (is_null_oid(&orig_head)) { if (merge_heads.nr > 1) die(_("Cannot merge multiple branches into empty head.")); - return pull_into_void(merge_heads.oid, &curr_head); + ret = pull_into_void(merge_heads.oid, &curr_head); + goto cleanup; } if (merge_heads.nr > 1) { if (opt_rebase) @@ -1125,8 +1127,6 @@ int cmd_pull(int argc, const char **argv, const char *prefix) } if (opt_rebase) { - int ret = 0; - struct object_id newbase; struct object_id upstream; get_rebase_newbase_and_upstream(&newbase, &upstream, &curr_head, @@ -1149,12 +1149,16 @@ int cmd_pull(int argc, const char **argv, const char *prefix) recurse_submodules == RECURSE_SUBMODULES_ON_DEMAND)) ret = rebase_submodules(); - return ret; + goto cleanup; } else { - int ret = run_merge(); + ret = run_merge(); if (!ret && (recurse_submodules == RECURSE_SUBMODULES_ON || recurse_submodules == RECURSE_SUBMODULES_ON_DEMAND)) ret = update_submodules(); - return ret; + goto cleanup; } + +cleanup: + oid_array_clear(&merge_heads); + return ret; } diff --git a/t/t5524-pull-msg.sh b/t/t5524-pull-msg.sh index b2be3605f5..56716e29dd 100755 --- a/t/t5524-pull-msg.sh +++ b/t/t5524-pull-msg.sh @@ -2,6 +2,7 @@ test_description='git pull message generation' +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh dollar='$Dollar' diff --git a/t/t6417-merge-ours-theirs.sh b/t/t6417-merge-ours-theirs.sh index 62d1406119..482b73a998 100755 --- a/t/t6417-merge-ours-theirs.sh +++ b/t/t6417-merge-ours-theirs.sh @@ -4,6 +4,7 @@ test_description='Merge-recursive ours and theirs variants' 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 ' diff --git a/t/t9101-git-svn-props.sh b/t/t9101-git-svn-props.sh index d043e80fc3..52046e60d5 100755 --- a/t/t9101-git-svn-props.sh +++ b/t/t9101-git-svn-props.sh @@ -5,7 +5,6 @@ test_description='git svn property tests' -TEST_FAILS_SANITIZE_LEAK=true . ./lib-git-svn.sh mkdir import