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 <ps@xxxxxx> 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); -- 2.46.0.46.g406f326d27.dirty