git-commit-vandalism/t/t7008-filter-branch-null-sha1.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

57 lines
1.4 KiB
Bash
Executable File

#!/bin/sh
test_description='filter-branch removal of trees with null sha1'
. ./test-lib.sh
test_expect_success 'setup: base commits' '
test_commit one &&
test_commit two &&
test_commit three
'
test_expect_success 'setup: a commit with a bogus null sha1 in the tree' '
{
git ls-tree HEAD &&
printf "160000 commit $ZERO_OID\\tbroken\\n"
} >broken-tree &&
echo "add broken entry" >msg &&
tree=$(git mktree <broken-tree) &&
test_tick &&
commit=$(git commit-tree $tree -p HEAD <msg) &&
git update-ref HEAD "$commit"
'
# we have to make one more commit on top removing the broken
# entry, since otherwise our index does not match HEAD (and filter-branch will
# complain). We could make the index match HEAD, but doing so would involve
# writing a null sha1 into the index.
test_expect_success 'setup: bring HEAD and index in sync' '
test_tick &&
git commit -a -m "back to normal"
'
test_expect_success 'noop filter-branch complains' '
test_must_fail git filter-branch \
--force --prune-empty \
--index-filter "true"
'
test_expect_success 'filter commands are still checked' '
test_must_fail git filter-branch \
--force --prune-empty \
--index-filter "git rm --cached --ignore-unmatch three.t"
'
test_expect_success 'removing the broken entry works' '
echo three >expect &&
git filter-branch \
--force --prune-empty \
--index-filter "git rm --cached --ignore-unmatch broken" &&
git log -1 --format=%s >actual &&
test_cmp expect actual
'
test_done