diff options
| -rw-r--r-- | cache.h | 17 | ||||
| -rw-r--r-- | name-hash.c | 182 | ||||
| -rw-r--r-- | read-cache.c | 9 | ||||
| -rwxr-xr-x | t/t7062-wtstatus-ignorecase.sh | 20 |
4 files changed, 166 insertions, 62 deletions
@@ -131,7 +131,6 @@ struct cache_entry { unsigned int ce_namelen; unsigned char sha1[20]; struct cache_entry *next; - struct cache_entry *dir_next; char name[FLEX_ARRAY]; /* more */ }; @@ -267,25 +266,15 @@ struct index_state { unsigned name_hash_initialized : 1, initialized : 1; struct hash_table name_hash; + struct hash_table dir_hash; }; extern struct index_state the_index; /* Name hashing */ extern void add_name_hash(struct index_state *istate, struct cache_entry *ce); -/* - * We don't actually *remove* it, we can just mark it invalid so that - * we won't find it in lookups. - * - * Not only would we have to search the lists (simple enough), but - * we'd also have to rehash other hash buckets in case this makes the - * hash bucket empty (common). So it's much better to just mark - * it. - */ -static inline void remove_name_hash(struct cache_entry *ce) -{ - ce->ce_flags |= CE_UNHASHED; -} +extern void remove_name_hash(struct index_state *istate, struct cache_entry *ce); +extern void free_name_hash(struct index_state *istate); #ifndef NO_THE_INDEX_COMPATIBILITY_MACROS diff --git a/name-hash.c b/name-hash.c index d8d25c23e9..91241336f8 100644 --- a/name-hash.c +++ b/name-hash.c @@ -32,38 +32,96 @@ static unsigned int hash_name(const char *name, int namelen) return hash; } -static void hash_index_entry_directories(struct index_state *istate, struct cache_entry *ce) +struct dir_entry { + struct dir_entry *next; + struct dir_entry *parent; + struct cache_entry *ce; + int nr; + unsigned int namelen; +}; + +static struct dir_entry *find_dir_entry(struct index_state *istate, + const char *name, unsigned int namelen) +{ + unsigned int hash = hash_name(name, namelen); + struct dir_entry *dir; + + for (dir = lookup_hash(hash, &istate->dir_hash); dir; dir = dir->next) + if (dir->namelen == namelen && + !strncasecmp(dir->ce->name, name, namelen)) + return dir; + return NULL; +} + +static struct dir_entry *hash_dir_entry(struct index_state *istate, + struct cache_entry *ce, int namelen) { /* * Throw each directory component in the hash for quick lookup * during a git status. Directory components are stored with their * closing slash. Despite submodules being a directory, they never * reach this point, because they are stored without a closing slash - * in the cache. + * in index_state.name_hash (as ordinary cache_entries). * - * Note that the cache_entry stored with the directory does not - * represent the directory itself. It is a pointer to an existing - * filename, and its only purpose is to represent existence of the - * directory in the cache. It is very possible multiple directory - * hash entries may point to the same cache_entry. + * Note that the cache_entry stored with the dir_entry merely + * supplies the name of the directory (up to dir_entry.namelen). We + * track the number of 'active' files in a directory in dir_entry.nr, + * so we can tell if the directory is still relevant, e.g. for git + * status. However, if cache_entries are removed, we cannot pinpoint + * an exact cache_entry that's still active. It is very possible that + * multiple dir_entries point to the same cache_entry. */ - unsigned int hash; - void **pos; + struct dir_entry *dir; + + /* get length of parent directory */ + while (namelen > 0 && !is_dir_sep(ce->name[namelen - 1])) + namelen--; + if (namelen <= 0) + return NULL; + + /* lookup existing entry for that directory */ + dir = find_dir_entry(istate, ce->name, namelen); + if (!dir) { + /* not found, create it and add to hash table */ + void **pdir; + unsigned int hash = hash_name(ce->name, namelen); - const char *ptr = ce->name; - while (*ptr) { - while (*ptr && *ptr != '/') - ++ptr; - if (*ptr == '/') { - ++ptr; - hash = hash_name(ce->name, ptr - ce->name); - pos = insert_hash(hash, ce, &istate->name_hash); - if (pos) { - ce->dir_next = *pos; - *pos = ce; - } + dir = xcalloc(1, sizeof(struct dir_entry)); + dir->namelen = namelen; + dir->ce = ce; + + pdir = insert_hash(hash, dir, &istate->dir_hash); + if (pdir) { + dir->next = *pdir; + *pdir = dir; } + + /* recursively add missing parent directories */ + dir->parent = hash_dir_entry(istate, ce, namelen - 1); } + return dir; +} + +static void add_dir_entry(struct index_state *istate, struct cache_entry *ce) +{ + /* Add reference to the directory entry (and parents if 0). */ + struct dir_entry *dir = hash_dir_entry(istate, ce, ce_namelen(ce)); + while (dir && !(dir->nr++)) + dir = dir->parent; +} + +static void remove_dir_entry(struct index_state *istate, struct cache_entry *ce) +{ + /* + * Release reference to the directory entry (and parents if 0). + * + * Note: we do not remove / free the entry because there's no + * hash.[ch]::remove_hash and dir->next may point to other entries + * that are still valid, so we must not free the memory. + */ + struct dir_entry *dir = hash_dir_entry(istate, ce, ce_namelen(ce)); + while (dir && dir->nr && !(--dir->nr)) + dir = dir->parent; } static void hash_index_entry(struct index_state *istate, struct cache_entry *ce) @@ -74,7 +132,7 @@ static void hash_index_entry(struct index_state *istate, struct cache_entry *ce) if (ce->ce_flags & CE_HASHED) return; ce->ce_flags |= CE_HASHED; - ce->next = ce->dir_next = NULL; + ce->next = NULL; hash = hash_name(ce->name, ce_namelen(ce)); pos = insert_hash(hash, ce, &istate->name_hash); if (pos) { @@ -82,8 +140,8 @@ static void hash_index_entry(struct index_state *istate, struct cache_entry *ce) *pos = ce; } - if (ignore_case) - hash_index_entry_directories(istate, ce); + if (ignore_case && !(ce->ce_flags & CE_UNHASHED)) + add_dir_entry(istate, ce); } static void lazy_init_name_hash(struct index_state *istate) @@ -99,11 +157,33 @@ static void lazy_init_name_hash(struct index_state *istate) void add_name_hash(struct index_state *istate, struct cache_entry *ce) { + /* if already hashed, add reference to directory entries */ + if (ignore_case && (ce->ce_flags & CE_STATE_MASK) == CE_STATE_MASK) + add_dir_entry(istate, ce); + ce->ce_flags &= ~CE_UNHASHED; if (istate->name_hash_initialized) hash_index_entry(istate, ce); } +/* + * We don't actually *remove* it, we can just mark it invalid so that + * we won't find it in lookups. + * + * Not only would we have to search the lists (simple enough), but + * we'd also have to rehash other hash buckets in case this makes the + * hash bucket empty (common). So it's much better to just mark + * it. + */ +void remove_name_hash(struct index_state *istate, struct cache_entry *ce) +{ + /* if already hashed, release reference to directory entries */ + if (ignore_case && (ce->ce_flags & CE_STATE_MASK) == CE_HASHED) + remove_dir_entry(istate, ce); + + ce->ce_flags |= CE_UNHASHED; +} + static int slow_same_name(const char *name1, int len1, const char *name2, int len2) { if (len1 != len2) @@ -137,18 +217,7 @@ static int same_name(const struct cache_entry *ce, const char *name, int namelen if (!icase) return 0; - /* - * If the entry we're comparing is a filename (no trailing slash), then compare - * the lengths exactly. - */ - if (name[namelen - 1] != '/') - return slow_same_name(name, namelen, ce->name, len); - - /* - * For a directory, we point to an arbitrary cache_entry filename. Just - * make sure the directory portion matches. - */ - return slow_same_name(name, namelen, ce->name, namelen < len ? namelen : len); + return slow_same_name(name, namelen, ce->name, len); } struct cache_entry *index_name_exists(struct index_state *istate, const char *name, int namelen, int icase) @@ -164,27 +233,54 @@ struct cache_entry *index_name_exists(struct index_state *istate, const char *na if (same_name(ce, name, namelen, icase)) return ce; } - if (icase && name[namelen - 1] == '/') - ce = ce->dir_next; - else - ce = ce->next; + ce = ce->next; } /* - * Might be a submodule. Despite submodules being directories, + * When looking for a directory (trailing '/'), it might be a + * submodule or a directory. Despite submodules being directories, * they are stored in the name hash without a closing slash. - * When ignore_case is 1, directories are stored in the name hash - * with their closing slash. + * When ignore_case is 1, directories are stored in a separate hash + * table *with* their closing slash. * * The side effect of this storage technique is we have need to + * lookup the directory in a separate hash table, and if not found * remove the slash from name and perform the lookup again without * the slash. If a match is made, S_ISGITLINK(ce->mode) will be * true. */ if (icase && name[namelen - 1] == '/') { + struct dir_entry *dir = find_dir_entry(istate, name, namelen); + if (dir && dir->nr) + return dir->ce; + ce = index_name_exists(istate, name, namelen - 1, icase); if (ce && S_ISGITLINK(ce->ce_mode)) return ce; } return NULL; } + +static int free_dir_entry(void *entry, void *unused) +{ + struct dir_entry *dir = entry; + while (dir) { + struct dir_entry *next = dir->next; + free(dir); + dir = next; + } + return 0; +} + +void free_name_hash(struct index_state *istate) +{ + if (!istate->name_hash_initialized) + return; + istate->name_hash_initialized = 0; + if (ignore_case) + /* free directory entries */ + for_each_hash(&istate->dir_hash, free_dir_entry, NULL); + + free_hash(&istate->name_hash); + free_hash(&istate->dir_hash); +} diff --git a/read-cache.c b/read-cache.c index 5cddbc325f..b4d08254d6 100644 --- a/read-cache.c +++ b/read-cache.c @@ -46,7 +46,7 @@ static void replace_index_entry(struct index_state *istate, int nr, struct cache { struct cache_entry *old = istate->cache[nr]; - remove_name_hash(old); + remove_name_hash(istate, old); set_index_entry(istate, nr, ce); istate->cache_changed = 1; } @@ -456,7 +456,7 @@ int remove_index_entry_at(struct index_state *istate, int pos) struct cache_entry *ce = istate->cache[pos]; record_resolve_undo(istate, ce); - remove_name_hash(ce); + remove_name_hash(istate, ce); istate->cache_changed = 1; istate->cache_nr--; if (pos >= istate->cache_nr) @@ -479,7 +479,7 @@ void remove_marked_cache_entries(struct index_state *istate) for (i = j = 0; i < istate->cache_nr; i++) { if (ce_array[i]->ce_flags & CE_REMOVE) - remove_name_hash(ce_array[i]); + remove_name_hash(istate, ce_array[i]); else ce_array[j++] = ce_array[i]; } @@ -1511,8 +1511,7 @@ int discard_index(struct index_state *istate) istate->cache_changed = 0; istate->timestamp.sec = 0; istate->timestamp.nsec = 0; - istate->name_hash_initialized = 0; - free_hash(&istate->name_hash); + free_name_hash(istate); cache_tree_free(&(istate->cache_tree)); istate->initialized = 0; diff --git a/t/t7062-wtstatus-ignorecase.sh b/t/t7062-wtstatus-ignorecase.sh new file mode 100755 index 0000000000..73709dbeee --- /dev/null +++ b/t/t7062-wtstatus-ignorecase.sh @@ -0,0 +1,20 @@ +#!/bin/sh + +test_description='git-status with core.ignorecase=true' + +. ./test-lib.sh + +test_expect_success 'status with hash collisions' ' + # note: "V/", "V/XQANY/" and "WURZAUP/" produce the same hash code + # in name-hash.c::hash_name + mkdir V && + mkdir V/XQANY && + mkdir WURZAUP && + touch V/XQANY/test && + git config core.ignorecase true && + git add . && + # test is successful if git status completes (no endless loop) + git status +' + +test_done |