[PATCH v4 00/22] Memory leak fixes (pt.4)
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Aug 14, 2024, 06:51 UTC
- Message-ID
- <cover.1723614263.git.ps@pks.im>
- In-Reply-To
- <cover.1722933642.git.ps@pks.im>
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@pks.im>
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