mbox series

[00/29] Memory leak fixes (pt.2)

Message ID cover.1717402439.git.ps@pks.im (mailing list archive)
Headers show
Series Memory leak fixes (pt.2) | expand

Message

Patrick Steinhardt June 3, 2024, 9:46 a.m. UTC 
Hi,

this is another random assortment of memory leak fixes for Git. With
these changes, another 77 test suites start to pass with the leak
sanitizer enabled. With that, we're down to ~270 test suites that do not
yet pass with the sanitizer enabled. My goal is that we can reduce this
number to 0 this year -- any help here would be appreciated.

While most of the commits are trivial, I realize that the overall series
is quite large. If you think that this is too big, please feel free to
speak up up and I'll split this and future series into batches of at
most 20 patches or less, if you think that's still too many.

The series is built on top of 9eaef5822c (Sync with 'maint', 2024-05-31)
with ps/leakfixes at 164937678c (Merge remote-tracking branch
'junio/ps/leakfixes' into HEAD, 2024-06-03) merged into it.

Thanks!

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 leaging 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                               | 110 +++++++++++++++-------
 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, 588 insertions(+), 269 deletions(-)

Comments

Karthik Nayak June 6, 2024, 2:33 p.m. UTC | #1 
Patrick Steinhardt <ps@pks.im> writes:

> Hi,
>
> this is another random assortment of memory leak fixes for Git. With
> these changes, another 77 test suites start to pass with the leak
> sanitizer enabled. With that, we're down to ~270 test suites that do not
> yet pass with the sanitizer enabled. My goal is that we can reduce this
> number to 0 this year -- any help here would be appreciated.
>

I was thinking of picking up some here, how do I avoid not clashing in
with the work that you're doing?

> While most of the commits are trivial, I realize that the overall series
> is quite large. If you think that this is too big, please feel free to
> speak up up and I'll split this and future series into batches of at
> most 20 patches or less, if you think that's still too many.
>
> The series is built on top of 9eaef5822c (Sync with 'maint', 2024-05-31)
> with ps/leakfixes at 164937678c (Merge remote-tracking branch
> 'junio/ps/leakfixes' into HEAD, 2024-06-03) merged into it.
>
> Thanks!
>

I only have some small typo nits apart from that this was a nice read. Thanks!
Patrick Steinhardt June 7, 2024, 4:07 a.m. UTC | #2 
On Thu, Jun 06, 2024 at 07:33:34AM -0700, Karthik Nayak wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > Hi,
> >
> > this is another random assortment of memory leak fixes for Git. With
> > these changes, another 77 test suites start to pass with the leak
> > sanitizer enabled. With that, we're down to ~270 test suites that do not
> > yet pass with the sanitizer enabled. My goal is that we can reduce this
> > number to 0 this year -- any help here would be appreciated.
> >
> 
> I was thinking of picking up some here, how do I avoid not clashing in
> with the work that you're doing?

You mean work that hasn't yet been posted to the mailing list? I don't
think there is a perfect answer here. I guess the next-best thing is to
not keep your patch series sitting for an extended amount of time, but
limit the time you are working on it and then send it out early such
that others can see it and ideally refrain from plugging the same leaks.

> > While most of the commits are trivial, I realize that the overall series
> > is quite large. If you think that this is too big, please feel free to
> > speak up up and I'll split this and future series into batches of at
> > most 20 patches or less, if you think that's still too many.
> >
> > The series is built on top of 9eaef5822c (Sync with 'maint', 2024-05-31)
> > with ps/leakfixes at 164937678c (Merge remote-tracking branch
> > 'junio/ps/leakfixes' into HEAD, 2024-06-03) merged into it.
> >
> > Thanks!
> >
> 
> I only have some small typo nits apart from that this was a nice read. Thanks!

Thanks!

Patrick