git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 00/10] sequencer API & users: fix widespread leaks

From
PWPhillip Wood <phillip.wood123@gmail.com>
Date
Dec 31, 2022, 15:06 UTC
Message-ID
<813dc31d-f36a-a4f0-ff99-2373df0eedac@dunelm.org.uk>
In-Reply-To
<cover-00.10-00000000000-20221230T071741Z-avarab@gmail.com>
Hi Ævar
On 30/12/2022 07:28, Ævar Arnfjörð Bjarmason wrote:
Show 15 quoted lines
> This series fixes various widespread leaks in the sequencer and its
> users (rebase, revert, cherry-pick). As a result 18 tests become
> leak-free in their entirety.
> 
> The main change is the 3/10 here, where we introduce a
> replay_opts_release() to free the "struct replay_opts". The rest is
> then either refactorings to be able to call that destructor
> (e.g. "return" to "goto cleanup"), or other miscellanious adjacent
> leaks.
> 
> This is a follow-up to the discussion ending at [1], as noted there
> the recent ff84d031a9d (Merge branch 'pw/rebase-no-reflog-action',
> 2022-11-23) ended up introducing a leak because of the disfunctional
> lack of a destructor (or rather, the current logic being tied up in
> sequencer_remove_state().

Thanks for working on this. Separating out replay_opts_release() from sequencer_remove_state() enables us to free struct replay_opts properly. I've left some comments, the patches I haven't commented on all looked fine.

Best Wishes
Phillip
Show 51 quoted lines
> This can be queued and graduated independently of the other concurrent
> leak series I've submitted[2]. When the two are combined we'll end up
> passing more tests, i.e. both topics combined get us over the finish
> line for some of them, but neither one is enough in isolation.
> 
> But that's OK, we just won't opt them into the "linux-leaks"
> testing. I'll submit a follow-up similar to [3] at some later date to
> mark them as passing. I think that's a better trade-off than making
> these two depend on one another.
> 
> 1. https://lore.kernel.org/git/221108.864jv9sc9r.gmgdl@evledraar.gmail.com/
> 2. https://lore.kernel.org/git/cover-v2-00.20-00000000000-20221230T020341Z-avarab@gmail.com/
> 3. https://lore.kernel.org/git/patch-v2-01.20-3de29c6d75f-20221230T020341Z-avarab@gmail.com/
> 
> Ævar Arnfjörð Bjarmason (10):
>    rebase: use "cleanup" pattern in do_interactive_rebase()
>    sequencer.c: split up sequencer_remove_state()
>    rebase & sequencer API: fix get_replay_opts() leak in "rebase"
>    builtin/revert.c: refactor run_sequencer() return pattern
>    builtin/revert.c: fix common leak by using replay_opts_release()
>    builtin/revert.c: move free-ing of "revs" to replay_opts_release()
>    builtin/rebase.c: fix "options.onto_name" leak
>    sequencer.c: always free() the "msgbuf" in do_pick_commit()
>    builtin/rebase.c: free() "options.strategy_opts"
>    commit.c: free() revs.commit in get_fork_point()
> 
>   builtin/rebase.c                       | 19 +++++-----
>   builtin/revert.c                       | 40 +++++++++++----------
>   commit.c                               |  1 +
>   sequencer.c                            | 48 +++++++++++++++++---------
>   sequencer.h                            |  1 +
>   t/t3405-rebase-malformed.sh            |  1 +
>   t/t3412-rebase-root.sh                 |  1 +
>   t/t3416-rebase-onto-threedots.sh       |  1 +
>   t/t3419-rebase-patch-id.sh             |  1 +
>   t/t3423-rebase-reword.sh               |  1 +
>   t/t3425-rebase-topology-merges.sh      |  2 ++
>   t/t3431-rebase-fork-point.sh           |  1 +
>   t/t3432-rebase-fast-forward.sh         |  1 +
>   t/t3437-rebase-fixup-options.sh        |  1 +
>   t/t3438-rebase-broken-files.sh         |  2 ++
>   t/t3501-revert-cherry-pick.sh          |  1 +
>   t/t3502-cherry-pick-merge.sh           |  1 +
>   t/t3503-cherry-pick-root.sh            |  1 +
>   t/t3506-cherry-pick-ff.sh              |  1 +
>   t/t3511-cherry-pick-x.sh               |  1 +
>   t/t7402-submodule-rebase.sh            |  1 +
>   t/t9106-git-svn-commit-diff-clobber.sh |  1 -
>   t/t9164-git-svn-dcommit-concurrent.sh  |  1 -
>   23 files changed, 82 insertions(+), 47 deletions(-)
> 
Previous: Ævar Arnfjörð BjarmasonNext: Junio C Hamano
Message 21 of 60 in “sequencer API & users: fix widespread leaks”
  1. 00/10 sequencer API & users: fix widespread leaksÆvar Arnfjörð Bjarmason, Dec 30, 2022
  2. 01/10 rebase: use "cleanup" pattern in do_interactive_rebase()Ævar Arnfjörð Bjarmason, Dec 30, 2022
  3. Phillip WoodDec 31, 2022
  4. 02/10 sequencer.c: split up sequencer_remove_state()Ævar Arnfjörð Bjarmason, Dec 30, 2022
  5. René ScharfeDec 30, 2022
  6. Phillip WoodDec 31, 2022
  7. 03/10 rebase & sequencer API: fix get_replay_opts() leak in "rebase"Ævar Arnfjörð Bjarmason, Dec 30, 2022
  8. Phillip WoodDec 31, 2022
  9. 04/10 builtin/revert.c: refactor run_sequencer() return patternÆvar Arnfjörð Bjarmason, Dec 30, 2022
  10. Junio C HamanoJan 1, 2023
  11. 05/10 builtin/revert.c: fix common leak by using replay_opts_release()Ævar Arnfjörð Bjarmason, Dec 30, 2022
  12. René ScharfeDec 30, 2022
  13. Phillip WoodDec 31, 2022
  14. 06/10 builtin/revert.c: move free-ing of "revs" to replay_opts_release()Ævar Arnfjörð Bjarmason, Dec 30, 2022
  15. 07/10 builtin/rebase.c: fix "options.onto_name" leakÆvar Arnfjörð Bjarmason, Dec 30, 2022
  16. Phillip WoodDec 31, 2022
  17. 08/10 sequencer.c: always free() the "msgbuf" in do_pick_commit()Ævar Arnfjörð Bjarmason, Dec 30, 2022
  18. Phillip WoodDec 31, 2022
  19. 09/10 builtin/rebase.c: free() "options.strategy_opts"Ævar Arnfjörð Bjarmason, Dec 30, 2022
  20. 10/10 commit.c: free() revs.commit in get_fork_point()Ævar Arnfjörð Bjarmason, Dec 30, 2022
  21. Phillip WoodDec 31, 2022
  22. Junio C HamanoJan 1, 2023
  23. 0/9 sequencer API & users: fix widespread leaksÆvar Arnfjörð Bjarmason, Jan 12, 2023
  24. 1/9 rebase: use "cleanup" pattern in do_interactive_rebase()Ævar Arnfjörð Bjarmason, Jan 12, 2023
  25. 2/9 sequencer.c: split up sequencer_remove_state()Ævar Arnfjörð Bjarmason, Jan 12, 2023
  26. 4/9 builtin/revert.c: move free-ing of "revs" to replay_opts_release()Ævar Arnfjörð Bjarmason, Jan 12, 2023
  27. Phillip WoodJan 13, 2023
  28. 3/9 rebase & sequencer API: fix get_replay_opts() leak in "rebase"Ævar Arnfjörð Bjarmason, Jan 12, 2023
  29. Phillip WoodJan 13, 2023
  30. 6/9 builtin/rebase.c: fix "options.onto_name" leakÆvar Arnfjörð Bjarmason, Jan 12, 2023
  31. Phillip WoodJan 13, 2023
  32. 5/9 builtin/rebase.c: rename "squash_onto_name" to "to_free"Ævar Arnfjörð Bjarmason, Jan 12, 2023
  33. Phillip WoodJan 13, 2023
  34. 7/9 sequencer.c: always free() the "msgbuf" in do_pick_commit()Ævar Arnfjörð Bjarmason, Jan 12, 2023
  35. 8/9 builtin/rebase.c: free() "options.strategy_opts"Ævar Arnfjörð Bjarmason, Jan 12, 2023
  36. 9/9 commit.c: free() revs.commit in get_fork_point()Ævar Arnfjörð Bjarmason, Jan 12, 2023
  37. Phillip WoodJan 13, 2023
  38. Junio C HamanoJan 13, 2023
  39. 0/8 sequencer API & users: fix widespread leaksÆvar Arnfjörð Bjarmason, Jan 18, 2023
  40. 1/8 rebase: use "cleanup" pattern in do_interactive_rebase()Ævar Arnfjörð Bjarmason, Jan 18, 2023
  41. 2/8 sequencer.c: split up sequencer_remove_state()Ævar Arnfjörð Bjarmason, Jan 18, 2023
  42. 3/8 rebase & sequencer API: fix get_replay_opts() leak in "rebase"Ævar Arnfjörð Bjarmason, Jan 18, 2023
  43. Phillip WoodJan 24, 2023
  44. 4/8 builtin/revert.c: move free-ing of "revs" to replay_opts_release()Ævar Arnfjörð Bjarmason, Jan 18, 2023
  45. 6/8 sequencer.c: always free() the "msgbuf" in do_pick_commit()Ævar Arnfjörð Bjarmason, Jan 18, 2023
  46. 8/8 commit.c: free() revs.commit in get_fork_point()Ævar Arnfjörð Bjarmason, Jan 18, 2023
  47. 7/8 builtin/rebase.c: free() "options.strategy_opts"Ævar Arnfjörð Bjarmason, Jan 18, 2023
  48. 0/8 sequencer API & users: fix widespread leaksÆvar Arnfjörð Bjarmason, Feb 6, 2023
  49. 1/8 rebase: use "cleanup" pattern in do_interactive_rebase()Ævar Arnfjörð Bjarmason, Feb 6, 2023
  50. 2/8 sequencer.c: split up sequencer_remove_state()Ævar Arnfjörð Bjarmason, Feb 6, 2023
  51. 3/8 sequencer API users: fix get_replay_opts() leaksÆvar Arnfjörð Bjarmason, Feb 6, 2023
  52. 4/8 builtin/revert.c: move free-ing of "revs" to replay_opts_release()Ævar Arnfjörð Bjarmason, Feb 6, 2023
  53. 5/8 builtin/rebase.c: fix "options.onto_name" leakÆvar Arnfjörð Bjarmason, Feb 6, 2023
  54. 7/8 builtin/rebase.c: free() "options.strategy_opts"Ævar Arnfjörð Bjarmason, Feb 6, 2023
  55. 6/8 sequencer.c: always free() the "msgbuf" in do_pick_commit()Ævar Arnfjörð Bjarmason, Feb 6, 2023
  56. 8/8 commit.c: free() revs.commit in get_fork_point()Ævar Arnfjörð Bjarmason, Feb 6, 2023
  57. Phillip WoodFeb 7, 2023
  58. 5/8 builtin/rebase.c: fix "options.onto_name" leakÆvar Arnfjörð Bjarmason, Jan 18, 2023
  59. Phillip WoodJan 24, 2023
  60. Phillip WoodJan 24, 2023

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.