[v4,00/22] Memory leak fixes (pt.4)

Message

Patrick Steinhardt Aug. 14, 2024, 6:51 a.m. UTC

Hi,

this is the fourth version of my fourth set of memory leak fixes. There
are only minor changes compared to v3:

  - Amend a commit message to explain that `release_revisions()` takes
    care of releasing `rev.diffopt` for us.

  - Fix another memory leak in `get_base_commit()`.

  - Remove `NULL` pointer check in `symdiff_release()` and stop
    zero-initializing `struct symdiff`.

Thanks!

Patrick

Patrick Steinhardt (22):
  remote: plug memory leak when aliasing URLs
  git: fix leaking system paths
  object-file: fix memory leak when reading corrupted headers
  object-name: fix leaking symlink paths in object context
  bulk-checkin: fix leaking state TODO
  read-cache: fix leaking hashfile when writing index fails
  submodule-config: fix leaking name entry when traversing submodules
  config: fix leaking comment character config
  builtin/rebase: fix leaking `commit.gpgsign` value
  builtin/notes: fix leaking `struct notes_tree` when merging notes
  builtin/fast-import: plug trivial memory leaks
  builtin/fast-export: fix leaking diff options
  builtin/fast-export: plug leaking tag names
  merge-ort: unconditionally release attributes index
  sequencer: release todo list on error paths
  unpack-trees: clear index when not propagating it
  diff: fix leak when parsing invalid ignore regex option
  builtin/format-patch: fix various trivial memory leaks
  userdiff: fix leaking memory for configured diff drivers
  builtin/log: fix leak when showing converted blob contents
  diff: free state populated via options
  builtin/diff: free symmetric diff members

 builtin/commit.c                      |  7 +-
 builtin/diff.c                        |  6 ++
 builtin/fast-export.c                 | 19 ++++--
 builtin/fast-import.c                 |  8 ++-
 builtin/log.c                         | 14 +++-
 builtin/notes.c                       |  9 ++-
 builtin/rebase.c                      |  1 +
 bulk-checkin.c                        |  2 +
 config.c                              |  3 +-
 csum-file.c                           |  2 +-
 csum-file.h                           | 10 +++
 diff.c                                | 16 ++++-
 environment.c                         |  1 +
 environment.h                         |  1 +
 git.c                                 | 13 +++-
 merge-ort.c                           |  3 +-
 object-file.c                         |  1 +
 object-name.c                         |  1 +
 range-diff.c                          |  6 +-
 read-cache.c                          | 97 ++++++++++++++++-----------
 remote.c                              |  2 +
 sequencer.c                           | 67 ++++++++++++------
 submodule-config.c                    | 18 +++--
 t/t0210-trace2-normal.sh              |  2 +-
 t/t1006-cat-file.sh                   |  1 +
 t/t1050-large.sh                      |  1 +
 t/t1450-fsck.sh                       |  1 +
 t/t1601-index-bogus.sh                |  2 +
 t/t2107-update-index-basic.sh         |  1 +
 t/t3310-notes-merge-manual-resolve.sh |  1 +
 t/t3311-notes-merge-fanout.sh         |  1 +
 t/t3404-rebase-interactive.sh         |  1 +
 t/t3435-rebase-gpg-sign.sh            |  1 +
 t/t3507-cherry-pick-conflict.sh       |  1 +
 t/t3510-cherry-pick-sequence.sh       |  1 +
 t/t3705-add-sparse-checkout.sh        |  1 +
 t/t4013-diff-various.sh               |  1 +
 t/t4014-format-patch.sh               |  1 +
 t/t4018-diff-funcname.sh              |  1 +
 t/t4030-diff-textconv.sh              |  2 +
 t/t4042-diff-textconv-caching.sh      |  2 +
 t/t4048-diff-combined-binary.sh       |  1 +
 t/t4064-diff-oidfind.sh               |  2 +
 t/t4065-diff-anchored.sh              |  1 +
 t/t4068-diff-symmetric-merge-base.sh  |  1 +
 t/t4069-remerge-diff.sh               |  1 +
 t/t4108-apply-threeway.sh             |  1 +
 t/t4209-log-pickaxe.sh                |  2 +
 t/t6421-merge-partial-clone.sh        |  1 +
 t/t6428-merge-conflicts-sparse.sh     |  1 +
 t/t7008-filter-branch-null-sha1.sh    |  1 +
 t/t7030-verify-tag.sh                 |  1 +
 t/t7817-grep-sparse-checkout.sh       |  1 +
 t/t9300-fast-import.sh                |  1 +
 t/t9304-fast-import-marks.sh          |  2 +
 t/t9351-fast-export-anonymize.sh      |  1 +
 unpack-trees.c                        |  2 +
 userdiff.c                            | 38 ++++++++---
 userdiff.h                            |  4 ++
 59 files changed, 286 insertions(+), 105 deletions(-)

Range-diff against v3:
 1:  02f6da020f =  1:  02f6da020f remote: plug memory leak when aliasing URLs
 2:  f36d895948 =  2:  f36d895948 git: fix leaking system paths
 3:  0415ac986d =  3:  0415ac986d object-file: fix memory leak when reading corrupted headers
 4:  e5130e50a9 =  4:  e5130e50a9 object-name: fix leaking symlink paths in object context
 5:  276c828ad1 =  5:  276c828ad1 bulk-checkin: fix leaking state TODO
 6:  ed0608e705 =  6:  ed0608e705 read-cache: fix leaking hashfile when writing index fails
 7:  b7a7f88c7d =  7:  b7a7f88c7d submodule-config: fix leaking name entry when traversing submodules
 8:  9054a459a1 =  8:  9054a459a1 config: fix leaking comment character config
 9:  1d3957a5eb =  9:  1d3957a5eb builtin/rebase: fix leaking `commit.gpgsign` value
10:  0af1bab5a1 = 10:  0af1bab5a1 builtin/notes: fix leaking `struct notes_tree` when merging notes
11:  30d4e9ed43 = 11:  30d4e9ed43 builtin/fast-import: plug trivial memory leaks
12:  9591fb7b5e ! 12:  070813a740 builtin/fast-export: fix leaking diff options
    @@ Commit message
     
         Before calling `handle_commit()` in a loop, we set `diffopt.no_free`
         such that its contents aren't getting freed inside of `handle_commit()`.
    -    We never unset that flag though, which means that it'll ultimately leak
    -    when calling `release_revisions()`.
    +    We never unset that flag though, which means that the structure's
    +    allocated resources will ultimately leak.
     
    -    Fix this by unsetting the flag after the loop.
    +    Fix this by unsetting the flag after the loop such that we release its
    +    resources via `release_revisions()`.
     
         Signed-off-by: Patrick Steinhardt <[e-mail]>
     
13:  254bbb7f6f = 13:  b4096e971f builtin/fast-export: plug leaking tag names
14:  334c4ed71a = 14:  bdfdf53313 merge-ort: unconditionally release attributes index
15:  9f08a859fb = 15:  f6c1055805 sequencer: release todo list on error paths
16:  5d4934b1a9 = 16:  9db41181a6 unpack-trees: clear index when not propagating it
17:  e1b6a24fbe = 17:  85f6ffd610 diff: fix leak when parsing invalid ignore regex option
18:  c048b54a2c ! 18:  e00aa1ef06 builtin/format-patch: fix various trivial memory leaks
    @@ Commit message
     
      ## builtin/log.c ##
     @@ builtin/log.c: static struct commit *get_base_commit(const struct format_config *cfg,
    + 				if (die_on_failure) {
    + 					die(_("failed to find exact merge base"));
    + 				} else {
    ++					free_commit_list(merge_base);
    + 					free(rev);
    + 					return NULL;
    + 				}
      			}
      
      			rev[i] = merge_base->item;
19:  39b2921e3e = 19:  cc04751134 userdiff: fix leaking memory for configured diff drivers
20:  50dea1c98a = 20:  0e2d3e523f builtin/log: fix leak when showing converted blob contents
21:  d5cb4ad580 = 21:  9faffa7a62 diff: free state populated via options
22:  31e38ba4e1 ! 22:  ee252e752c builtin/diff: free symmetric diff members
    @@ builtin/diff.c: static void symdiff_prepare(struct rev_info *rev, struct symdiff
      
     +static void symdiff_release(struct symdiff *sdiff)
     +{
    -+	if (!sdiff)
    -+		return;
     +	bitmap_free(sdiff->skip);
     +}
     +
      int cmd_diff(int argc, const char **argv, const char *prefix)
      {
      	int i;
    -@@ builtin/diff.c: int cmd_diff(int argc, const char **argv, const char *prefix)
    - 	struct object_array_entry *blob[2];
    - 	int nongit = 0, no_index = 0;
    - 	int result;
    --	struct symdiff sdiff;
    -+	struct symdiff sdiff = {0};
    - 
    - 	/*
    - 	 * We could get N tree-ish in the rev.pending_objects list.
     @@ builtin/diff.c: int cmd_diff(int argc, const char **argv, const char *prefix)
      		refresh_index_quietly();
      	release_revisions(&rev);