Patrick Steinhardt <ps@xxxxxx> writes:
> Hi,
>
> this is the second version of my patch series that fixes various memory
> leaks in our codebase.
>
> Changes compared to v1:
>
> - Fix various typos in commit messages.
>
> - Refactor patch 20 such that we do not have both `ret` and `res` as
> local variables, which was confusing.
>
> Thanks!
>
The changes with the range-diff looks good to me, Thanks!
> Patrick
>
> Patrick Steinhardt (29):
> revision: fix memory leak when reversing revisions
> parse-options: fix leaks for users of OPT_FILENAME
> notes-utils: free note trees when releasing copied notes
> bundle: plug leaks in `create_bundle()`
> biultin/rev-parse: fix memory leaks in `--parseopt` mode
> merge-recursive: fix leaking rename conflict info
> revision: fix leaking display notes
> notes: fix memory leak when pruning notes
> builtin/rev-list: fix leaking bitmap index when calculating disk usage
> object-name: free leaking object contexts
> builtin/difftool: plug memory leaks in `run_dir_diff()`
> builtin/merge-recursive: fix leaking object ID bases
> merge-recursive: fix memory leak when finalizing merge
> builtin/log: fix leaking commit list in git-cherry(1)
> revision: free diff options
> builtin/stash: fix leak in `show_stash()`
> rerere: fix various trivial leaks
> config: fix leaking "core.notesref" variable
> commit: fix leaking parents when calling `commit_tree_extended()`
> sequencer: fix leaking string buffer in `commit_staged_changes()`
> apply: fix leaking string in `match_fragment()`
> builtin/clone: plug leaking HEAD ref in `wanted_peer_refs()`
> sequencer: fix memory leaks in `make_script_with_merges()`
> builtin/merge: fix leaking `struct cmdnames` in `get_strategy()`
> merge: fix leaking merge bases
> line-range: plug leaking find functions
> blame: fix leaking data for blame scoreboards
> builtin/blame: fix leaking prefixed paths
> builtin/blame: fix leaking ignore revs files
>
> apply.c | 88 ++++++++++------
> apply.h | 2 +-
> blame.c | 4 +
> builtin/am.c | 7 +-
> builtin/archive.c | 7 +-
> builtin/blame.c | 8 +-
> builtin/cat-file.c | 17 ++--
> builtin/clone.c | 3 +-
> builtin/commit-tree.c | 11 +-
> builtin/commit.c | 10 +-
> builtin/difftool.c | 3 +
> builtin/fmt-merge-msg.c | 4 +-
> builtin/grep.c | 4 +-
> builtin/log.c | 16 +--
> builtin/ls-tree.c | 3 +-
> builtin/merge-recursive.c | 6 +-
> builtin/merge-tree.c | 1 +
> builtin/merge.c | 18 +++-
> builtin/multi-pack-index.c | 13 ++-
> builtin/replay.c | 14 ++-
> builtin/rev-list.c | 2 +
> builtin/rev-parse.c | 55 +++++-----
> builtin/shortlog.c | 5 +-
> builtin/sparse-checkout.c | 1 +
> builtin/stash.c | 23 +++--
> bundle.c | 29 ++++--
> commit.c | 28 +++---
> commit.h | 12 +--
> config.c | 1 +
> diff-lib.c | 2 +
> diff.c | 8 +-
> help.c | 12 +--
> help.h | 2 +
> line-range.c | 2 +
> list-objects-filter.c | 2 +
> log-tree.c | 1 +
> merge-ort-wrappers.c | 2 +-
> merge-ort-wrappers.h | 2 +-
> merge-ort.c | 12 ++-
> merge-ort.h | 2 +-
> merge-recursive.c | 68 ++++++++-----
> merge-recursive.h | 4 +-
> notes-merge.c | 1 +
> notes-utils.c | 9 +-
> notes-utils.h | 2 +-
> notes.c | 21 ++--
> notes.h | 5 +
> object-name.c | 40 ++++++--
> object-name.h | 2 +
> rerere.c | 3 +
> revision.c | 59 +++++++----
> sequencer.c | 116 +++++++++++++++-------
> t/helper/test-parse-options.c | 1 +
> t/t1004-read-tree-m-u-wf.sh | 1 +
> t/t1015-read-index-unmerged.sh | 2 +
> t/t1021-rerere-in-workdir.sh | 1 +
> t/t1512-rev-parse-disambiguation.sh | 1 +
> t/t2500-untracked-overwriting.sh | 1 +
> t/t3301-notes.sh | 1 +
> t/t3306-notes-prune.sh | 1 +
> t/t3308-notes-merge.sh | 1 +
> t/t3309-notes-merge-auto-resolve.sh | 1 +
> t/t3400-rebase.sh | 1 +
> t/t3401-rebase-and-am-rename.sh | 1 +
> t/t3403-rebase-skip.sh | 1 +
> t/t3406-rebase-message.sh | 1 +
> t/t3407-rebase-abort.sh | 1 +
> t/t3417-rebase-whitespace-fix.sh | 1 +
> t/t3418-rebase-continue.sh | 1 +
> t/t3420-rebase-autostash.sh | 1 +
> t/t3421-rebase-topology-linear.sh | 2 +
> t/t3424-rebase-empty.sh | 1 +
> t/t3428-rebase-signoff.sh | 1 +
> t/t3430-rebase-merges.sh | 1 +
> t/t3434-rebase-i18n.sh | 1 +
> t/t3500-cherry.sh | 1 +
> t/t3504-cherry-pick-rerere.sh | 1 +
> t/t3505-cherry-pick-empty.sh | 1 +
> t/t3508-cherry-pick-many-commits.sh | 1 +
> t/t3509-cherry-pick-merge-df.sh | 1 +
> t/t3907-stash-show-config.sh | 1 +
> t/t4061-diff-indent.sh | 1 +
> t/t4131-apply-fake-ancestor.sh | 1 +
> t/t4151-am-abort.sh | 1 +
> t/t4153-am-resume-override-opts.sh | 1 +
> t/t4208-log-magic-pathspec.sh | 1 +
> t/t4253-am-keep-cr-dos.sh | 1 +
> t/t4255-am-submodule.sh | 1 +
> t/t5150-request-pull.sh | 1 +
> t/t5300-pack-object.sh | 4 +-
> t/t5305-include-tag.sh | 1 +
> t/t5407-post-rewrite-hook.sh | 1 +
> t/t5605-clone-local.sh | 1 +
> t/t5607-clone-bundle.sh | 1 +
> t/t5612-clone-refspec.sh | 1 +
> t/t6000-rev-list-misc.sh | 1 +
> t/t6001-rev-list-graft.sh | 1 +
> t/t6013-rev-list-reverse-parents.sh | 1 +
> t/t6017-rev-list-stdin.sh | 1 +
> t/t6020-bundle-misc.sh | 1 +
> t/t6115-rev-list-du.sh | 2 +
> t/t6130-pathspec-noglob.sh | 2 +
> t/t6402-merge-rename.sh | 1 +
> t/t6427-diff3-conflict-markers.sh | 1 +
> t/t6430-merge-recursive.sh | 1 +
> t/t6432-merge-recursive-space-options.sh | 1 +
> t/t6434-merge-recursive-rename-options.sh | 1 +
> t/t6436-merge-overwrite.sh | 1 +
> t/t7006-pager.sh | 1 +
> t/t7010-setup.sh | 1 +
> t/t7012-skip-worktree-writing.sh | 1 +
> t/t7201-co.sh | 1 +
> t/t7501-commit-basic-functionality.sh | 1 +
> t/t7505-prepare-commit-msg-hook.sh | 1 +
> t/t7512-status-help.sh | 1 +
> t/t7600-merge.sh | 1 +
> t/t7606-merge-custom.sh | 1 +
> t/t7611-merge-abort.sh | 1 +
> t/t8002-blame.sh | 1 +
> t/t8003-blame-corner-cases.sh | 1 +
> t/t8004-blame-with-conflicts.sh | 1 +
> t/t8006-blame-textconv.sh | 2 +
> t/t8008-blame-formats.sh | 2 +
> t/t8009-blame-vs-topicbranches.sh | 2 +
> t/t8011-blame-split-file.sh | 2 +
> t/t8012-blame-colors.sh | 1 +
> t/t8013-blame-ignore-revs.sh | 2 +
> t/t8014-blame-ignore-fuzzy.sh | 2 +
> t/t9500-gitweb-standalone-no-errors.sh | 1 +
> t/t9502-gitweb-standalone-parse-output.sh | 1 +
> 130 files changed, 591 insertions(+), 272 deletions(-)
>
> Range-diff against v1:
> 1: a88a6fa266 = 1: f602bb5444 revision: fix memory leak when reversing revisions
> 2: ecabbb74e1 = 2: 84f2f56b71 parse-options: fix leaks for users of OPT_FILENAME
> 3: 17a8f53275 ! 3: c5f49bf930 notes-utils: free note trees when releasing copied notes
> @@ Metadata
> ## Commit message ##
> notes-utils: free note trees when releasing copied notes
>
> - While we clear most of the members of `struct notet_rewrite_cfg` in
> + While we clear most of the members of `struct notes_rewrite_cfg` in
> `finish_copy_notes_for_rewrite()`, we do not clear the notes tree. Fix
> this to plug this memory leak.
>
> 4: 0c8c97cded = 4: 5a1eae2cc9 bundle: plug leaks in `create_bundle()`
> 5: 620814fb99 = 5: 91eefcf64a biultin/rev-parse: fix memory leaks in `--parseopt` mode
> 6: 0052d60ee8 ! 6: f16c29a20a merge-recursive: fix leaging rename conflict info
> @@ Metadata
> Author: Patrick Steinhardt <ps@xxxxxx>
>
> ## Commit message ##
> - merge-recursive: fix leaging rename conflict info
> + merge-recursive: fix leaking rename conflict info
>
> When computing rename conflicts in our recursive merge algorithm we set
> up `struct rename_conflict_info`s to track that information. We never
> free those data structures though and thus leak memory.
>
> We need to be a bit more careful here though because the same rename
> - conflict info can be assigned to multiple structures. Accomodate for
> + conflict info can be assigned to multiple structures. Accommodate for
> this by introducing a `rename_conflict_info_owned` bit that we can use
> to steer whether or not the rename conflict info shall be free'd.
>
> 7: 8c55370b8e = 7: b9c3a34ae1 revision: fix leaking display notes
> 8: 17671dc9cf = 8: 3ea5e5c6bd notes: fix memory leak when pruning notes
> 9: 5e01c39e35 = 9: a0242359b7 builtin/rev-list: fix leaking bitmap index when calculating disk usage
> 10: 2ac24bf942 = 10: bdb7389328 object-name: free leaking object contexts
> 11: 8349f6acbd = 11: e6ff9a3f1c builtin/difftool: plug memory leaks in `run_dir_diff()`
> 12: 5d9fb78e63 = 12: fb30a254ab builtin/merge-recursive: fix leaking object ID bases
> 13: 930de11911 = 13: b6540e4aff merge-recursive: fix memory leak when finalizing merge
> 14: 3e3243f884 = 14: 7cb4f75178 builtin/log: fix leaking commit list in git-cherry(1)
> 15: a754800fd1 = 15: 99f4f3d341 revision: free diff options
> 16: 8bde010d1d = 16: 707d798f78 builtin/stash: fix leak in `show_stash()`
> 17: cadeb09b95 = 17: 7d73c50872 rerere: fix various trivial leaks
> 18: cff6303ce1 = 18: 1700ec8d21 config: fix leaking "core.notesref" variable
> 19: 8ada8864d8 = 19: c1a1eb34ab commit: fix leaking parents when calling `commit_tree_extended()`
> 20: 48bcd0ac80 ! 20: 144eb23617 sequencer: fix leaking string buffer in `commit_staged_changes()`
> @@ sequencer.c: static int commit_staged_changes(struct repository *r,
> /*
> * If a fixup/squash in a fixup/squash chain failed, the
> @@ sequencer.c: static int commit_staged_changes(struct repository *r,
> + * We need to update the squash message to skip
> + * the latest commit message.
> + */
> +- int res = 0;
> + struct commit *commit;
> + const char *msg;
> const char *path = rebase_path_squash_msg();
> const char *encoding = get_commit_output_encoding();
>
> @@ sequencer.c: static int commit_staged_changes(struct repository *r,
>
> p = repo_logmsg_reencode(r, commit, NULL, encoding);
> if (!p) {
> -@@ sequencer.c: static int commit_staged_changes(struct repository *r,
> +- res = error(_("could not parse commit %s"),
> ++ ret = error(_("could not parse commit %s"),
> + oid_to_hex(&commit->object.oid));
> + goto unuse_commit_buffer;
> + }
> + find_commit_subject(p, &msg);
> + if (write_message(msg, strlen(msg), path, 0)) {
> +- res = error(_("could not write file: "
> ++ ret = error(_("could not write file: "
> + "'%s'"), path);
> + goto unuse_commit_buffer;
> }
> ++
> ++ ret = 0;
> ++
> unuse_commit_buffer:
> repo_unuse_commit_buffer(r, commit, p);
> - if (res)
> - return res;
> -+ if (res) {
> -+ ret = res;
> ++ if (ret)
> + goto out;
> -+ }
> }
> }
>
> 21: ebed18717f = 21: 8e1cf8a18b apply: fix leaking string in `match_fragment()`
> 22: 4f4ae5ea96 = 22: 8090891bd0 builtin/clone: plug leaking HEAD ref in `wanted_peer_refs()`
> 23: 713531c6f1 = 23: 1cdd988c45 sequencer: fix memory leaks in `make_script_with_merges()`
> 24: eeef280dd5 = 24: be9bccc853 builtin/merge: fix leaking `struct cmdnames` in `get_strategy()`
> 25: 5441f1e2ef = 25: 6f6456494c merge: fix leaking merge bases
> 26: ba9c2b4ea7 = 26: 67f8637dc6 line-range: plug leaking find functions
> 27: 5f2b0adc56 = 27: bd6db56a3f blame: fix leaking data for blame scoreboards
> 28: 7ee8734437 = 28: c1291025fa builtin/blame: fix leaking prefixed paths
> 29: 5b5fdfc6cb = 29: 5d7007e724 builtin/blame: fix leaking ignore revs files
> --
> 2.45.2.436.gcd77e87115.dirty
Attachment:
signature.asc
Description: PGP signature
