Re: [PATCH v3 4/8] unpack-trees: fix nested sparse-dir search
From: Elijah Newren <hidden>
Date: 2021-08-19 18:29:17
On Tue, Aug 17, 2021 at 6:23 AM Derrick Stolee via GitGitGadget [off-list ref] wrote:
From: Derrick Stolee <redacted> The iterated search in find_cache_entry() was recently modified to include a loop that searches backwards for a sparse directory entry that matches the given traverse_info and name_entry. However, the string comparison failed to actually concatenate those two strings, so this failed to find a sparse directory when it was not a top-level directory. This caused some errors in rare cases where a 'git checkout' spanned a diff that modified files within the sparse directory entry, but we could not correctly find the entry.
Doh, sorry for missing that in my review of the earlier series.
quoted hunk ↗ jump to hunk
Signed-off-by: Derrick Stolee <redacted> --- unpack-trees.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-)diff --git a/unpack-trees.c b/unpack-trees.c index 5786645f315..df1f4437723 100644 --- a/unpack-trees.c +++ b/unpack-trees.c@@ -1255,9 +1255,10 @@ static int sparse_dir_matches_path(const struct cache_entry *ce, static struct cache_entry *find_cache_entry(struct traverse_info *info, const struct name_entry *p) { - struct cache_entry *ce; + struct cache_entry *ce = NULL; int pos = find_cache_pos(info, p->path, p->pathlen); struct unpack_trees_options *o = info->data; + struct strbuf full_path = STRBUF_INIT; if (0 <= pos) return o->src_index->cache[pos];@@ -1273,6 +1274,10 @@ static struct cache_entry *find_cache_entry(struct traverse_info *info, if (pos < 0 || pos >= o->src_index->cache_nr) return NULL; + strbuf_addstr(&full_path, info->traverse_path); + strbuf_add(&full_path, p->path, p->pathlen); + strbuf_addch(&full_path, '/'); + /* * Due to lexicographic sorting and sparse directory * entries ending with a trailing slash, our path as a@@ -1283,17 +1288,20 @@ static struct cache_entry *find_cache_entry(struct traverse_info *info, while (pos >= 0) { ce = o->src_index->cache[pos]; - if (strncmp(ce->name, p->path, p->pathlen)) - return NULL; + if (strncmp(ce->name, full_path.buf, full_path.len)) { + ce = NULL; + break; + } if (S_ISSPARSEDIR(ce->ce_mode) && sparse_dir_matches_path(ce, info, p)) - return ce; + break; pos--; } - return NULL; + strbuf_release(&full_path); + return ce; } static void debug_path(struct traverse_info *info) --gitgitgadget
Makes sense.