path: root/path.c
diff options
authorSZEDER Gábor <>2019-10-21 16:00:43 (GMT)
committerJunio C Hamano <>2019-10-23 03:54:22 (GMT)
commitf45f88b2e483649cd063a7dc7826c03025683e56 (patch)
treef3efec05ab50f32e1e3fcb85d48f4e74132a2b68 /path.c
parentc72fc40d0904cbf3199258c2f471c2351e024b1e (diff)
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 <> Signed-off-by: Junio C Hamano <>
Diffstat (limited to 'path.c')
1 files changed, 6 insertions, 5 deletions
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;