git/t/t4066-diff-emit-delay.sh
Ævar Arnfjörð Bjarmason a52f07afcb revisions API: have release_revisions() release "mailmap"
Extend the the release_revisions() function so that it frees the
"mailmap" in the "struct rev_info".

The log family of functions now calls the clear_mailmap() function
added in fa8afd18e5a (revisions API: provide and use a
release_revisions(), 2021-09-19), allowing us to whitelist some tests
with "TEST_PASSES_SANITIZE_LEAK=true".

Unfortunately having a pointer to a mailmap in "struct rev_info"
instead of an embedded member that we "own" get a bit messy, as can be
seen in the change to builtin/commit.c.

When we free() this data we won't be able to tell apart a pointer to a
"mailmap" on the heap from one on the stack. As seen in
ea57bc0d41 (log: add --use-mailmap option, 2013-01-05) the "log"
family allocates it on the heap, but in the find_author_by_nickname()
code added in ea16794e43 (commit: search author pattern against
mailmap, 2013-08-23) we allocated it on the stack instead.

Ideally we'd simply change that member to a "struct string_list
mailmap" and never free() the "mailmap" itself, but that would be a
much larger change to the revisions API.

We have code that needs to hand an existing "mailmap" to a "struct
rev_info", while we could change all of that, let's not go there
now.

The complexity isn't in the ownership of the "mailmap" per-se, but
that various things assume a "rev_info.mailmap == NULL" means "doesn't
want mailmap", if we changed that to an init'd "struct string_list
we'd need to carefully refactor things to change those assumptions.

Let's instead always free() it, and simply declare that if you add
such a "mailmap" it must be allocated on the heap. Any modern libc
will correctly panic if we free() a stack variable, so this should be
safe going forward.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-04-13 23:56:09 -07:00

84 lines
1.7 KiB
Bash
Executable File

#!/bin/sh
test_description='test combined/stat/moved interaction'
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
TEST_PASSES_SANITIZE_LEAK=true
. ./test-lib.sh
# This test covers a weird 3-way interaction between "--cc -p", which will run
# the combined diff code, along with "--stat", which will be computed as a
# first-parent stat during the combined diff, and "--color-moved", which
# enables the emitted_symbols list to store the diff in memory.
test_expect_success 'set up history with a merge' '
test_commit A &&
test_commit B &&
git checkout -b side HEAD^ &&
test_commit C &&
git merge -m M main &&
test_commit D
'
test_expect_success 'log --cc -p --stat --color-moved' '
cat >expect <<-EOF &&
commit D
---
D.t | 1 +
1 file changed, 1 insertion(+)
diff --git a/D.t b/D.t
new file mode 100644
index 0000000..$(git rev-parse --short D:D.t)
--- /dev/null
+++ b/D.t
@@ -0,0 +1 @@
+D
commit M
B.t | 1 +
1 file changed, 1 insertion(+)
commit C
---
C.t | 1 +
1 file changed, 1 insertion(+)
diff --git a/C.t b/C.t
new file mode 100644
index 0000000..$(git rev-parse --short C:C.t)
--- /dev/null
+++ b/C.t
@@ -0,0 +1 @@
+C
commit B
---
B.t | 1 +
1 file changed, 1 insertion(+)
diff --git a/B.t b/B.t
new file mode 100644
index 0000000..$(git rev-parse --short B:B.t)
--- /dev/null
+++ b/B.t
@@ -0,0 +1 @@
+B
commit A
---
A.t | 1 +
1 file changed, 1 insertion(+)
diff --git a/A.t b/A.t
new file mode 100644
index 0000000..$(git rev-parse --short A:A.t)
--- /dev/null
+++ b/A.t
@@ -0,0 +1 @@
+A
EOF
git log --format="commit %s" --cc -p --stat --color-moved >actual &&
test_cmp expect actual
'
test_done