[PATCH v2 00/11] built-ins: fix common memory leaks

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

Fix common memory leaks in built-ins, for a general overview see the
v1 CL:
https://lore.kernel.org/git/cover-00.11-00000000000-20220630T175714Z-avarab@gmail.com/

Changes since v1:

 * Amend the commit message of 2/11, we always get copy_ref() data,
   but note that we do so via an indirection in clone.c
 * Replace 8/11, maybe that solution is going overboard, we could also
   just drop it from this series...

Ævar Arnfjörð Bjarmason (11):
  check-ref-format: fix trivial memory leak
  clone: fix memory leak in wanted_peer_refs()
  submodule.c: free() memory from xgetcwd()
  revert: free "struct replay_opts" members
  cat-file: fix a memory leak in --batch-command mode
  merge-file: refactor for subsequent memory leak fix
  merge-file: fix memory leaks on error path
  checkout: avoid "struct unpack_trees_options" leak
  gc: fix a memory leak
  cat-file: fix a common "struct object_context" memory leak
  pull: fix a "struct oid_array" memory leak

 builtin/cat-file.c                   | 33 +++++++++++++++++--------
 builtin/check-ref-format.c           | 11 ++++++---
 builtin/checkout.c                   | 36 +++++++++++++++++-----------
 builtin/clone.c                      |  1 +
 builtin/gc.c                         |  8 ++++++-
 builtin/merge-file.c                 | 32 ++++++++++++++-----------
 builtin/pull.c                       | 16 ++++++++-----
 builtin/revert.c                     |  3 +++
 submodule.c                          |  3 ++-
 t/t0028-working-tree-encoding.sh     |  1 +
 t/t1051-large-conversion.sh          |  2 ++
 t/t1402-check-ref-format.sh          |  1 +
 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/t5524-pull-msg.sh                  |  1 +
 t/t6403-merge-file.sh                |  2 ++
 t/t6417-merge-ours-theirs.sh         |  1 +
 t/t6422-merge-rename-corner-cases.sh |  1 +
 t/t8007-cat-file-textconv.sh         |  2 ++
 t/t8010-cat-file-filters.sh          |  2 ++
 t/t9101-git-svn-props.sh             |  1 -
 t/t9104-git-svn-follow-parent.sh     |  1 -
 t/t9132-git-svn-broken-symlink.sh    |  1 -
 t/t9301-fast-import-notes.sh         |  1 +
 26 files changed, 114 insertions(+), 54 deletions(-)

Range-diff against v1:
 1:  f35cf7c6ee9 =  1:  f35cf7c6ee9 check-ref-format: fix trivial memory leak
 2:  8e424238071 !  2:  24a1eaa51a3 clone: fix memory leak in copy_ref() call
    @@ Metadata
     Author: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
      ## Commit message ##
    -    clone: fix memory leak in copy_ref() call
    +    clone: fix memory leak in wanted_peer_refs()
     
         Fix a memory leak added in 0ec4b1650cc (clone: fix ref selection in
    -    --single-branch --branch=xxx, 2012-06-22), we need to free_refs() the
    -    result of copy_ref().
    +    --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
 3:  a687d1281f8 =  3:  691b178aaf0 submodule.c: free() memory from xgetcwd()
 4:  c9c2380be34 =  4:  6fc895676f4 revert: free "struct replay_opts" members
 5:  9ba267377ee =  5:  692d6e0e3d8 cat-file: fix a memory leak in --batch-command mode
 6:  17e66130b94 =  6:  d894e97b5ae merge-file: refactor for subsequent memory leak fix
 7:  8803a0df799 =  7:  8658f3ad020 merge-file: fix memory leaks on error path
 8:  94e28aa7ab3 !  8:  e21d7e4e9df checkout: add a missing clear_unpack_trees_porcelain()
    @@ Metadata
     Author: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
      ## Commit message ##
    -    checkout: add a missing clear_unpack_trees_porcelain()
    +    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, let's also clear what we've
    -    allocated in that case.
    +    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>
     
      ## builtin/checkout.c ##
    +@@ builtin/checkout.c: 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,
     @@ builtin/checkout.c: 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()) {
    -+			clear_unpack_trees_porcelain(&topts);
    - 			error(_("you need to resolve your current index first"));
    - 			return 1;
    +@@ builtin/checkout.c: 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 :
 9:  b51a275b6b1 =  9:  10a97a9cac4 gc: fix a memory leak
10:  9474e2bcf93 = 10:  481d006378b cat-file: fix a common "struct object_context" memory leak
11:  022399ad652 = 11:  5666104943f pull: fix a "struct oid_array" memory leak
-- 
2.37.0.900.g4d0de1cceb2