git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "René Scharfe" <l.s.r@web.de>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>, git@vger.kernel.org
Cc: Phillip Wood <phillip.wood@dunelm.org.uk>,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	Junio C Hamano <gitster@pobox.com>, Taylor Blau <me@ttaylorr.com>
Subject: Re: [PATCH 05/10] builtin/revert.c: fix common leak by using replay_opts_release()
Date: Sat, 31 Dec 2022 00:37:48 +0100	[thread overview]
Message-ID: <3a823d76-9ea3-d6a0-c9ed-9e84fe57042c@web.de> (raw)
In-Reply-To: <patch-05.10-e2895bb9795-20221230T071741Z-avarab@gmail.com>

Am 30.12.22 um 08:28 schrieb Ævar Arnfjörð Bjarmason:
> Use the replay_opts_release() function introduced in a preceding
> commit to fix a memory leak in run_sequencer().
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  builtin/revert.c                  | 1 +
>  t/t3419-rebase-patch-id.sh        | 1 +
>  t/t3425-rebase-topology-merges.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 +
>  8 files changed, 9 insertions(+)
>
> diff --git a/builtin/revert.c b/builtin/revert.c
> index e956d125a2b..2f656b25619 100644
> --- a/builtin/revert.c
> +++ b/builtin/revert.c
> @@ -240,6 +240,7 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)
>  		cmd == 's' ? sequencer_skip :
>  		sequencer_pick_revisions;
>  	ret = cbfun(the_repository, opts);
> +	replay_opts_release(opts);

Is this the right place for this call?  opts is passed in by this function's
two callers.  They should clean it up instead, no?  That would add one line,
but avoid the need for patch 4.  Would make patch 6 easier to read as well.

>  	return ret;
>  }
>
> diff --git a/t/t3419-rebase-patch-id.sh b/t/t3419-rebase-patch-id.sh
> index 7181f176b81..6c61f240cf9 100755
> --- a/t/t3419-rebase-patch-id.sh
> +++ b/t/t3419-rebase-patch-id.sh
> @@ -5,6 +5,7 @@ test_description='git rebase - test patch id computation'
>  GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
>  export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
>
> +TEST_PASSES_SANITIZE_LEAK=true
>  . ./test-lib.sh
>
>  scramble () {
> diff --git a/t/t3425-rebase-topology-merges.sh b/t/t3425-rebase-topology-merges.sh
> index 63acc1ea4da..a16428bdf54 100755
> --- a/t/t3425-rebase-topology-merges.sh
> +++ b/t/t3425-rebase-topology-merges.sh
> @@ -1,6 +1,8 @@
>  #!/bin/sh
>
>  test_description='rebase topology tests with merges'
> +
> +TEST_PASSES_SANITIZE_LEAK=true
>  . ./test-lib.sh
>  . "$TEST_DIRECTORY"/lib-rebase.sh
>
> diff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh
> index 1f4cfc37449..2f3e3e24169 100755
> --- a/t/t3501-revert-cherry-pick.sh
> +++ b/t/t3501-revert-cherry-pick.sh
> @@ -13,6 +13,7 @@ test_description='test cherry-pick and revert with renames
>  GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
>  export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
>
> +TEST_PASSES_SANITIZE_LEAK=true
>  . ./test-lib.sh
>
>  test_expect_success setup '
> diff --git a/t/t3502-cherry-pick-merge.sh b/t/t3502-cherry-pick-merge.sh
> index 5495eacfec1..1b2c0d6aca6 100755
> --- a/t/t3502-cherry-pick-merge.sh
> +++ b/t/t3502-cherry-pick-merge.sh
> @@ -11,6 +11,7 @@ test_description='cherry picking and reverting a merge
>  GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
>  export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
>
> +TEST_PASSES_SANITIZE_LEAK=true
>  . ./test-lib.sh
>
>  test_expect_success setup '
> diff --git a/t/t3503-cherry-pick-root.sh b/t/t3503-cherry-pick-root.sh
> index 95fe4feaeee..76d393dc8a3 100755
> --- a/t/t3503-cherry-pick-root.sh
> +++ b/t/t3503-cherry-pick-root.sh
> @@ -5,6 +5,7 @@ test_description='test cherry-picking (and reverting) a root commit'
>  GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
>  export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
>
> +TEST_PASSES_SANITIZE_LEAK=true
>  . ./test-lib.sh
>
>  test_expect_success setup '
> diff --git a/t/t3506-cherry-pick-ff.sh b/t/t3506-cherry-pick-ff.sh
> index 7e11bd4a4c5..b71bad17b85 100755
> --- a/t/t3506-cherry-pick-ff.sh
> +++ b/t/t3506-cherry-pick-ff.sh
> @@ -5,6 +5,7 @@ test_description='test cherry-picking with --ff option'
>  GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
>  export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
>
> +TEST_PASSES_SANITIZE_LEAK=true
>  . ./test-lib.sh
>
>  test_expect_success setup '
> diff --git a/t/t3511-cherry-pick-x.sh b/t/t3511-cherry-pick-x.sh
> index 84a587daf3a..dd5d92ef302 100755
> --- a/t/t3511-cherry-pick-x.sh
> +++ b/t/t3511-cherry-pick-x.sh
> @@ -2,6 +2,7 @@
>
>  test_description='Test cherry-pick -x and -s'
>
> +TEST_PASSES_SANITIZE_LEAK=true
>  . ./test-lib.sh
>
>  pristine_detach () {


  reply	other threads:[~2022-12-30 23:38 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-30  7:28 [PATCH 00/10] sequencer API & users: fix widespread leaks Ævar Arnfjörð Bjarmason
2022-12-30  7:28 ` [PATCH 01/10] rebase: use "cleanup" pattern in do_interactive_rebase() Ævar Arnfjörð Bjarmason
2022-12-31 14:50   ` Phillip Wood
2022-12-30  7:28 ` [PATCH 02/10] sequencer.c: split up sequencer_remove_state() Ævar Arnfjörð Bjarmason
2022-12-30 17:35   ` René Scharfe
2022-12-31 14:51     ` Phillip Wood
2022-12-30  7:28 ` [PATCH 03/10] rebase & sequencer API: fix get_replay_opts() leak in "rebase" Ævar Arnfjörð Bjarmason
2022-12-31 14:54   ` Phillip Wood
2022-12-30  7:28 ` [PATCH 04/10] builtin/revert.c: refactor run_sequencer() return pattern Ævar Arnfjörð Bjarmason
2023-01-01  4:25   ` Junio C Hamano
2022-12-30  7:28 ` [PATCH 05/10] builtin/revert.c: fix common leak by using replay_opts_release() Ævar Arnfjörð Bjarmason
2022-12-30 23:37   ` René Scharfe [this message]
2022-12-31 14:55     ` Phillip Wood
2022-12-30  7:28 ` [PATCH 06/10] builtin/revert.c: move free-ing of "revs" to replay_opts_release() Ævar Arnfjörð Bjarmason
2022-12-30  7:28 ` [PATCH 07/10] builtin/rebase.c: fix "options.onto_name" leak Ævar Arnfjörð Bjarmason
2022-12-31 14:59   ` Phillip Wood
2022-12-30  7:28 ` [PATCH 08/10] sequencer.c: always free() the "msgbuf" in do_pick_commit() Ævar Arnfjörð Bjarmason
2022-12-31 15:03   ` Phillip Wood
2022-12-30  7:28 ` [PATCH 09/10] builtin/rebase.c: free() "options.strategy_opts" Ævar Arnfjörð Bjarmason
2022-12-30  7:28 ` [PATCH 10/10] commit.c: free() revs.commit in get_fork_point() Ævar Arnfjörð Bjarmason
2022-12-31 15:06 ` [PATCH 00/10] sequencer API & users: fix widespread leaks Phillip Wood
2023-01-01  4:27   ` Junio C Hamano
2023-01-12 12:45 ` [PATCH v2 0/9] " Ævar Arnfjörð Bjarmason
2023-01-12 12:45   ` [PATCH v2 1/9] rebase: use "cleanup" pattern in do_interactive_rebase() Ævar Arnfjörð Bjarmason
2023-01-12 12:45   ` [PATCH v2 2/9] sequencer.c: split up sequencer_remove_state() Ævar Arnfjörð Bjarmason
2023-01-12 12:45   ` [PATCH v2 3/9] rebase & sequencer API: fix get_replay_opts() leak in "rebase" Ævar Arnfjörð Bjarmason
2023-01-13 10:34     ` Phillip Wood
2023-01-12 12:45   ` [PATCH v2 4/9] builtin/revert.c: move free-ing of "revs" to replay_opts_release() Ævar Arnfjörð Bjarmason
2023-01-13 10:36     ` Phillip Wood
2023-01-12 12:45   ` [PATCH v2 5/9] builtin/rebase.c: rename "squash_onto_name" to "to_free" Ævar Arnfjörð Bjarmason
2023-01-13 10:37     ` Phillip Wood
2023-01-12 12:45   ` [PATCH v2 6/9] builtin/rebase.c: fix "options.onto_name" leak Ævar Arnfjörð Bjarmason
2023-01-13 10:41     ` Phillip Wood
2023-01-12 12:45   ` [PATCH v2 7/9] sequencer.c: always free() the "msgbuf" in do_pick_commit() Ævar Arnfjörð Bjarmason
2023-01-12 12:46   ` [PATCH v2 8/9] builtin/rebase.c: free() "options.strategy_opts" Ævar Arnfjörð Bjarmason
2023-01-12 12:46   ` [PATCH v2 9/9] commit.c: free() revs.commit in get_fork_point() Ævar Arnfjörð Bjarmason
2023-01-13 10:45   ` [PATCH v2 0/9] sequencer API & users: fix widespread leaks Phillip Wood
2023-01-13 20:47     ` Junio C Hamano
2023-01-18 16:09   ` [PATCH v3 0/8] " Ævar Arnfjörð Bjarmason
2023-01-18 16:09     ` [PATCH v3 1/8] rebase: use "cleanup" pattern in do_interactive_rebase() Ævar Arnfjörð Bjarmason
2023-01-18 16:09     ` [PATCH v3 2/8] sequencer.c: split up sequencer_remove_state() Ævar Arnfjörð Bjarmason
2023-01-18 16:09     ` [PATCH v3 3/8] rebase & sequencer API: fix get_replay_opts() leak in "rebase" Ævar Arnfjörð Bjarmason
2023-01-24 14:36       ` Phillip Wood
2023-01-18 16:09     ` [PATCH v3 4/8] builtin/revert.c: move free-ing of "revs" to replay_opts_release() Ævar Arnfjörð Bjarmason
2023-01-18 16:09     ` [PATCH v3 5/8] builtin/rebase.c: fix "options.onto_name" leak Ævar Arnfjörð Bjarmason
2023-01-24 14:40       ` Phillip Wood
2023-01-18 16:09     ` [PATCH v3 6/8] sequencer.c: always free() the "msgbuf" in do_pick_commit() Ævar Arnfjörð Bjarmason
2023-01-18 16:09     ` [PATCH v3 7/8] builtin/rebase.c: free() "options.strategy_opts" Ævar Arnfjörð Bjarmason
2023-02-06 19:08       ` [PATCH v4 0/8] sequencer API & users: fix widespread leaks Ævar Arnfjörð Bjarmason
2023-02-06 19:08         ` [PATCH v4 1/8] rebase: use "cleanup" pattern in do_interactive_rebase() Ævar Arnfjörð Bjarmason
2023-02-06 19:08         ` [PATCH v4 2/8] sequencer.c: split up sequencer_remove_state() Ævar Arnfjörð Bjarmason
2023-02-06 19:08         ` [PATCH v4 3/8] sequencer API users: fix get_replay_opts() leaks Ævar Arnfjörð Bjarmason
2023-02-06 19:08         ` [PATCH v4 4/8] builtin/revert.c: move free-ing of "revs" to replay_opts_release() Ævar Arnfjörð Bjarmason
2023-02-06 19:08         ` [PATCH v4 5/8] builtin/rebase.c: fix "options.onto_name" leak Ævar Arnfjörð Bjarmason
2023-02-06 19:08         ` [PATCH v4 6/8] sequencer.c: always free() the "msgbuf" in do_pick_commit() Ævar Arnfjörð Bjarmason
2023-02-06 19:08         ` [PATCH v4 7/8] builtin/rebase.c: free() "options.strategy_opts" Ævar Arnfjörð Bjarmason
2023-02-06 19:08         ` [PATCH v4 8/8] commit.c: free() revs.commit in get_fork_point() Ævar Arnfjörð Bjarmason
2023-02-07 10:19         ` [PATCH v4 0/8] sequencer API & users: fix widespread leaks Phillip Wood
2023-01-18 16:09     ` [PATCH v3 8/8] commit.c: free() revs.commit in get_fork_point() Ævar Arnfjörð Bjarmason
2023-01-24 14:41     ` [PATCH v3 0/8] sequencer API & users: fix widespread leaks Phillip Wood

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3a823d76-9ea3-d6a0-c9ed-9e84fe57042c@web.de \
    --to=l.s.r@web.de \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=me@ttaylorr.com \
    --cc=phillip.wood@dunelm.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).