@matheusbsbs / matheus-bagliano / commits / 33d0dda633

checkout: avoid "struct unpack_trees_options" leak

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 <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Ævar Arnfjörð Bjarmason committed Jul 1, 2022 at 12:42 UTC 33d0dda633f3748e1af517135a72141f5df35f2d
1 file changed +22 -14
builtin/checkout.c
+22 -14
@@ -710,6 +710,26 @@ static void setup_branch_path(struct branch_info *branch)
710 branch->path = strbuf_detach(&buf, NULL);
711 }
712
713 +static void init_topts(struct unpack_trees_options *topts, int merge,
714 + int show_progress, int overwrite_ignore,
715 + struct commit *old_commit)
716 +{
717 + memset(topts, 0, sizeof(*topts));
718 + topts->head_idx = -1;
719 + topts->src_index = &the_index;
720 + topts->dst_index = &the_index;
721 +
722 + setup_unpack_trees_porcelain(topts, "checkout");
723 +
724 + topts->initial_checkout = is_cache_unborn();
725 + topts->update = 1;
726 + topts->merge = 1;
727 + topts->quiet = merge && old_commit;
728 + topts->verbose_update = show_progress;
729 + topts->fn = twoway_merge;
730 + topts->preserve_ignored = !overwrite_ignore;
731 +}
732 +
733 static int merge_working_tree(const struct checkout_opts *opts,
734 struct branch_info *old_branch_info,
735 struct branch_info *new_branch_info,
@@ -740,13 +760,6 @@ static int merge_working_tree(const struct checkout_opts *opts,
760 struct unpack_trees_options topts;
761 const struct object_id *old_commit_oid;
762
743 - memset(&topts, 0, sizeof(topts));
744 - topts.head_idx = -1;
745 - topts.src_index = &the_index;
746 - topts.dst_index = &the_index;
747 -
748 - setup_unpack_trees_porcelain(&topts, "checkout");
749 -
763 refresh_cache(REFRESH_QUIET);
764
765 if (unmerged_cache()) {
@@ -755,17 +768,12 @@ static int merge_working_tree(const struct checkout_opts *opts,
768 }
769
770 /* 2-way merge to the new branch */
758 - topts.initial_checkout = is_cache_unborn();
759 - topts.update = 1;
760 - topts.merge = 1;
761 - topts.quiet = opts->merge && old_branch_info->commit;
762 - topts.verbose_update = opts->show_progress;
763 - topts.fn = twoway_merge;
771 + init_topts(&topts, opts->merge, opts->show_progress,
772 + opts->overwrite_ignore, old_branch_info->commit);
773 init_checkout_metadata(&topts.meta, new_branch_info->refname,
774 new_branch_info->commit ?
775 &new_branch_info->commit->object.oid :
776 &new_branch_info->oid, NULL);
768 - topts.preserve_ignored = !opts->overwrite_ignore;
777
778 old_commit_oid = old_branch_info->commit ?
779 &old_branch_info->commit->object.oid :