mirror/git
1
0
Fork 0
mirror of https://github.com/git/git.git synced 2024-09-21 17:51:40 +02:00
Code

path.c: don't call the match function without value in trie_find()

Browse Source 
'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>
This commit is contained in:
SZEDER Gábor 2019-10-21 18:00:43 +02:00 committed by Junio C Hamano
parent c72fc40d09
commit f45f88b2e4
2 changed files with 8 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
path.c
View File

@ -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;

2
t/t0060-path-utils.sh
View File

@ -288,6 +288,8 @@ test_git_path GIT_COMMON_DIR=bar index .git/index
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