path.c: don't call the match function without value in trie_find()
'logs/refs' is not a working tree-specific path, but since commit
b9317d55a3 (Make sure refs/rewritten/ is per-worktree, 2019-03-07)
'git rev-parse --git-path' has been returning a bogus path if a
trailing '/' is present:
$ git -C WT/ rev-parse --git-path logs/refs --git-path logs/refs/
/home/szeder/src/git/.git/logs/refs
/home/szeder/src/git/.git/worktrees/WT/logs/refs/
We use a trie data structure to efficiently decide whether a path
belongs to the common dir or is working tree-specific. As it happens
b9317d55a3 triggered a bug that is as old as the trie implementation
itself, added in 4e09cf2acf (path: optimize common dir checking,
2015-08-31).
- According to the comment describing trie_find(), it should only
call the given match function 'fn' for a "/-or-\0-terminated
prefix of the key for which the trie contains a value". This is
not true: there are three places where trie_find() calls the match
function, but one of them is missing the check for value's
existence.
- b9317d55a3 added two new keys to the trie: 'logs/refs/rewritten'
and 'logs/refs/worktree', next to the already existing
'logs/refs/bisect'. This resulted in a trie node with the path
'logs/refs/', which didn't exist before, and which doesn't have a
value attached. A query for 'logs/refs/' finds this node and then
hits that one callsite of the match function which doesn't check
for the value's existence, and thus invokes the match function
with NULL as value.
- When the match function check_common() is invoked with a NULL
value, it returns 0, which indicates that the queried path doesn't
belong to the common directory, ultimately resulting the bogus
path shown above.
Add the missing condition to trie_find() so it will never invoke the
match function with a non-existing value. check_common() will then no
longer have to check that it got a non-NULL value, so remove that
condition.
I believe that there are no other paths that could cause similar bogus
output. AFAICT the only other key resulting in the match function
being called with a NULL value is 'co' (because of the keys 'common'
and 'config'). However, as they are not in a directory that belongs
to the common directory the resulting working tree-specific path is
expected.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
diff --git a/path.c b/path.c
index 81e9bfe..4b69360 100644
--- a/path.c
+++ b/path.c
@@ -299,9 +299,13 @@ static int trie_find(struct trie *root, const char *key, match_fn fn,
/* Matched the entire compressed section */
key += i;
- if (!*key)
+ if (!*key) {
/* End of key */
- return fn(key, root->value, baton);
+ if (root->value)
+ return fn(key, root->value, baton);
+ else
+ return -1;
+ }
/* Partial path normalization: skip consecutive slashes */
while (key[0] == '/' && key[1] == '/')
@@ -345,9 +349,6 @@ static int check_common(const char *unmatched, void *value, void *baton)
{
struct common_dir *dir = value;
- if (!dir)
- return 0;
-
if (dir->is_dir && (unmatched[0] == 0 || unmatched[0] == '/'))
return dir->is_common;
diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
index c7b53e4..501e1a2 100755
--- a/t/t0060-path-utils.sh
+++ b/t/t0060-path-utils.sh
@@ -288,6 +288,8 @@
test_git_path GIT_COMMON_DIR=bar HEAD .git/HEAD
test_git_path GIT_COMMON_DIR=bar logs/HEAD .git/logs/HEAD
test_git_path GIT_COMMON_DIR=bar logs/refs/bisect/foo .git/logs/refs/bisect/foo
+test_git_path GIT_COMMON_DIR=bar logs/refs bar/logs/refs
+test_git_path GIT_COMMON_DIR=bar logs/refs/ bar/logs/refs/
test_git_path GIT_COMMON_DIR=bar logs/refs/bisec/foo bar/logs/refs/bisec/foo
test_git_path GIT_COMMON_DIR=bar logs/refs/bisec bar/logs/refs/bisec
test_git_path GIT_COMMON_DIR=bar logs/refs/bisectfoo bar/logs/refs/bisectfoo
