From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
"Phillip Wood" <phillip.wood123@gmail.com>,
"Jeff King" <peff@peff.net>,
"Derrick Stolee" <derrickstolee@github.com>,
"Elijah Newren" <newren@gmail.com>,
"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH 12/17] sequencer.c: fix a pick_commits() leak
Date: Thu, 3 Nov 2022 18:06:11 +0100 [thread overview]
Message-ID: <patch-12.17-7429151b5c4-20221103T164632Z-avarab@gmail.com> (raw)
In-Reply-To: <cover-00.17-00000000000-20221103T164632Z-avarab@gmail.com>
Fix a leak introduced in 1f6965f994f (sequencer: honor
GIT_REFLOG_ACTION, 2020-04-07), we should free() the xstrdup()'d
"prev_reflog_action" string.
We only needed to xstrdup() the previous action if we took this branch
within the "while" statement, and needed to set the GIT_REFLOG_ACTION
for the duration of the do_pick_commit(). Let's just query and restore
it there instead.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
sequencer.c | 9 ++++++---
t/t2012-checkout-last.sh | 1 +
t/t3409-rebase-environ.sh | 1 +
t/t3413-rebase-hook.sh | 1 +
t/t3433-rebase-across-mode-change.sh | 1 +
t/t7504-commit-msg-hook.sh | 1 +
t/t9115-git-svn-dcommit-funky-renames.sh | 1 -
t/t9146-git-svn-empty-dirs.sh | 1 -
t/t9160-git-svn-preserve-empty-dirs.sh | 1 -
9 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 07d7062bfb8..14ca0af2ade 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -4567,11 +4567,9 @@ static int pick_commits(struct repository *r,
struct replay_opts *opts)
{
int res = 0, reschedule = 0;
- char *prev_reflog_action;
/* Note that 0 for 3rd parameter of setenv means set only if not set */
setenv(GIT_REFLOG_ACTION, action_name(opts), 0);
- prev_reflog_action = xstrdup(getenv(GIT_REFLOG_ACTION));
if (opts->allow_ff)
assert(!(opts->signoff || opts->no_commit ||
opts->record_origin || should_edit(opts) ||
@@ -4618,15 +4616,20 @@ static int pick_commits(struct repository *r,
}
}
if (item->command <= TODO_SQUASH) {
- if (is_rebase_i(opts))
+ char *prev_reflog_action = NULL;
+
+ if (is_rebase_i(opts)) {
+ prev_reflog_action = xstrdup(getenv(GIT_REFLOG_ACTION));
setenv(GIT_REFLOG_ACTION, reflog_message(opts,
command_to_string(item->command), NULL),
1);
+ }
res = do_pick_commit(r, item, opts,
is_final_fixup(todo_list),
&check_todo);
if (is_rebase_i(opts))
setenv(GIT_REFLOG_ACTION, prev_reflog_action, 1);
+ free(prev_reflog_action);
if (is_rebase_i(opts) && res < 0) {
/* Reschedule */
advise(_(rescheduled_advice),
diff --git a/t/t2012-checkout-last.sh b/t/t2012-checkout-last.sh
index 1f6c4ed0428..4b6372f4c3e 100755
--- a/t/t2012-checkout-last.sh
+++ b/t/t2012-checkout-last.sh
@@ -5,6 +5,7 @@ test_description='checkout can switch to last branch and merge base'
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/t3409-rebase-environ.sh b/t/t3409-rebase-environ.sh
index 83ffb39d9ff..acaf5558dbe 100755
--- a/t/t3409-rebase-environ.sh
+++ b/t/t3409-rebase-environ.sh
@@ -2,6 +2,7 @@
test_description='git rebase interactive environment'
+TEST_PASSES_SANITIZE_LEAK=true
. ./test-lib.sh
test_expect_success 'setup' '
diff --git a/t/t3413-rebase-hook.sh b/t/t3413-rebase-hook.sh
index 9fab0d779bb..e8456831e8b 100755
--- a/t/t3413-rebase-hook.sh
+++ b/t/t3413-rebase-hook.sh
@@ -5,6 +5,7 @@ test_description='git rebase with its hook(s)'
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/t3433-rebase-across-mode-change.sh b/t/t3433-rebase-across-mode-change.sh
index 05df964670f..c8172b08522 100755
--- a/t/t3433-rebase-across-mode-change.sh
+++ b/t/t3433-rebase-across-mode-change.sh
@@ -2,6 +2,7 @@
test_description='git rebase across mode change'
+TEST_PASSES_SANITIZE_LEAK=true
. ./test-lib.sh
test_expect_success 'setup' '
diff --git a/t/t7504-commit-msg-hook.sh b/t/t7504-commit-msg-hook.sh
index a39de8c1126..07ca46fb0d5 100755
--- a/t/t7504-commit-msg-hook.sh
+++ b/t/t7504-commit-msg-hook.sh
@@ -5,6 +5,7 @@ test_description='commit-msg hook'
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 'with no hook' '
diff --git a/t/t9115-git-svn-dcommit-funky-renames.sh b/t/t9115-git-svn-dcommit-funky-renames.sh
index 419f055721d..743fbe1fe46 100755
--- a/t/t9115-git-svn-dcommit-funky-renames.sh
+++ b/t/t9115-git-svn-dcommit-funky-renames.sh
@@ -5,7 +5,6 @@
test_description='git svn dcommit can commit renames of files with ugly names'
-TEST_FAILS_SANITIZE_LEAK=true
. ./lib-git-svn.sh
test_expect_success 'load repository with strange names' '
diff --git a/t/t9146-git-svn-empty-dirs.sh b/t/t9146-git-svn-empty-dirs.sh
index 79c26ed69c1..09606f1b3cf 100755
--- a/t/t9146-git-svn-empty-dirs.sh
+++ b/t/t9146-git-svn-empty-dirs.sh
@@ -4,7 +4,6 @@
test_description='git svn creates empty directories'
-TEST_FAILS_SANITIZE_LEAK=true
. ./lib-git-svn.sh
test_expect_success 'initialize repo' '
diff --git a/t/t9160-git-svn-preserve-empty-dirs.sh b/t/t9160-git-svn-preserve-empty-dirs.sh
index 9cf7a1427ab..36c6b1a12ff 100755
--- a/t/t9160-git-svn-preserve-empty-dirs.sh
+++ b/t/t9160-git-svn-preserve-empty-dirs.sh
@@ -9,7 +9,6 @@ This test uses git to clone a Subversion repository that contains empty
directories, and checks that corresponding directories are created in the
local Git repository with placeholder files.'
-TEST_FAILS_SANITIZE_LEAK=true
. ./lib-git-svn.sh
GIT_REPO=git-svn-repo
--
2.38.0.1451.g86b35f4140a
next prev parent reply other threads:[~2022-11-03 17:07 UTC|newest]
Thread overview: 45+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-03 17:05 [PATCH 00/17] leak fixes: use existing constructors & other trivia Ævar Arnfjörð Bjarmason
2022-11-03 17:06 ` [PATCH 01/17] tests: mark tests as passing with SANITIZE=leak Ævar Arnfjörð Bjarmason
2022-11-03 17:06 ` [PATCH 02/17] {reset,merge}: call discard_index() before returning Ævar Arnfjörð Bjarmason
2022-11-03 17:06 ` [PATCH 03/17] commit: discard partial cache before (re-)reading it Ævar Arnfjörð Bjarmason
2022-11-03 17:06 ` [PATCH 04/17] read-cache.c: clear and free "sparse_checkout_patterns" Ævar Arnfjörð Bjarmason
2022-11-03 17:06 ` [PATCH 05/17] dir.c: free "ident" and "exclude_per_dir" in "struct untracked_cache" Ævar Arnfjörð Bjarmason
2022-11-03 17:06 ` [PATCH 06/17] built-ins & libs & helpers: add/move destructors, fix leaks Ævar Arnfjörð Bjarmason
2022-11-03 17:06 ` [PATCH 07/17] unpack-file: fix ancient leak in create_temp_file() Ævar Arnfjörð Bjarmason
2022-11-03 17:06 ` [PATCH 08/17] revision API: call graph_clear() in release_revisions() Ævar Arnfjörð Bjarmason
2022-11-03 17:06 ` [PATCH 09/17] ls-files: fix a --with-tree memory leak Ævar Arnfjörð Bjarmason
2022-11-03 17:06 ` [PATCH 10/17] sequencer.c: fix "opts->strategy" leak in read_strategy_opts() Ævar Arnfjörð Bjarmason
2022-11-04 14:50 ` Phillip Wood
2022-11-04 21:44 ` Taylor Blau
2022-11-05 12:43 ` Ævar Arnfjörð Bjarmason
2022-11-08 15:00 ` Phillip Wood
2022-11-08 15:26 ` Ævar Arnfjörð Bjarmason
2022-11-03 17:06 ` [PATCH 11/17] connected.c: free the "struct packed_git" Ævar Arnfjörð Bjarmason
2022-11-03 17:06 ` Ævar Arnfjörð Bjarmason [this message]
2022-11-03 17:06 ` [PATCH 13/17] rebase: don't leak on "--abort" Ævar Arnfjörð Bjarmason
2022-11-04 14:42 ` Phillip Wood
2022-11-05 12:01 ` Ævar Arnfjörð Bjarmason
2022-11-03 17:06 ` [PATCH 14/17] sequencer.c: fix sequencer_continue() leak Ævar Arnfjörð Bjarmason
2022-11-03 17:06 ` [PATCH 15/17] cherry-pick: free "struct replay_opts" members Ævar Arnfjörð Bjarmason
2022-11-03 17:06 ` [PATCH 16/17] revert: fix parse_options_concat() leak Ævar Arnfjörð Bjarmason
2022-11-03 17:06 ` [PATCH 17/17] built-ins: use free() not UNLEAK() if trivial, rm dead code Ævar Arnfjörð Bjarmason
2022-11-04 15:20 ` [PATCH 00/17] leak fixes: use existing constructors & other trivia Phillip Wood
2022-11-05 12:46 ` Ævar Arnfjörð Bjarmason
2022-11-07 9:46 ` Phillip Wood
2022-11-08 18:17 ` [PATCH v2 00/15] " Ævar Arnfjörð Bjarmason
2022-11-08 18:17 ` [PATCH v2 01/15] tests: mark tests as passing with SANITIZE=leak Ævar Arnfjörð Bjarmason
2022-11-08 18:17 ` [PATCH v2 02/15] {reset,merge}: call discard_index() before returning Ævar Arnfjörð Bjarmason
2022-11-08 18:17 ` [PATCH v2 03/15] commit: discard partial cache before (re-)reading it Ævar Arnfjörð Bjarmason
2022-11-08 18:17 ` [PATCH v2 04/15] read-cache.c: clear and free "sparse_checkout_patterns" Ævar Arnfjörð Bjarmason
2022-11-08 18:17 ` [PATCH v2 05/15] dir.c: free "ident" and "exclude_per_dir" in "struct untracked_cache" Ævar Arnfjörð Bjarmason
2022-11-08 18:17 ` [PATCH v2 06/15] built-ins & libs & helpers: add/move destructors, fix leaks Ævar Arnfjörð Bjarmason
2022-11-08 18:17 ` [PATCH v2 07/15] unpack-file: fix ancient leak in create_temp_file() Ævar Arnfjörð Bjarmason
2022-11-08 18:17 ` [PATCH v2 08/15] revision API: call graph_clear() in release_revisions() Ævar Arnfjörð Bjarmason
2022-11-08 18:17 ` [PATCH v2 09/15] ls-files: fix a --with-tree memory leak Ævar Arnfjörð Bjarmason
2022-11-08 18:17 ` [PATCH v2 10/15] sequencer.c: fix "opts->strategy" leak in read_strategy_opts() Ævar Arnfjörð Bjarmason
2022-11-08 18:17 ` [PATCH v2 11/15] connected.c: free the "struct packed_git" Ævar Arnfjörð Bjarmason
2022-11-08 18:17 ` [PATCH v2 12/15] rebase: don't leak on "--abort" Ævar Arnfjörð Bjarmason
2022-11-08 18:17 ` [PATCH v2 13/15] cherry-pick: free "struct replay_opts" members Ævar Arnfjörð Bjarmason
2022-11-08 18:17 ` [PATCH v2 14/15] revert: fix parse_options_concat() leak Ævar Arnfjörð Bjarmason
2022-11-08 18:17 ` [PATCH v2 15/15] built-ins: use free() not UNLEAK() if trivial, rm dead code Ævar Arnfjörð Bjarmason
2022-11-08 20:54 ` [PATCH v2 00/15] leak fixes: use existing constructors & other trivia Taylor Blau
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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=patch-12.17-7429151b5c4-20221103T164632Z-avarab@gmail.com \
--to=avarab@gmail.com \
--cc=derrickstolee@github.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=newren@gmail.com \
--cc=peff@peff.net \
--cc=phillip.wood123@gmail.com \
/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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.