Re: [PATCH v3 4/8] unpack-trees: fix nested sparse-dir search
- From
Elijah Newren <newren@gmail.com>
- Date
- Aug 19, 2021, 18:29 UTC
- Message-ID
- <CABPp-BG4gdX7G-a=jthfGVR_TJ=OxC210Ed6++rMkDwUe2QbiQ@mail.gmail.com>
- In-Reply-To
- <10bcadb284e49419f9b4baf75e05f719ec395d98.1629206603.git.gitgitgadget@gmail.com>
On Tue, Aug 17, 2021 at 6:23 AM Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com> wrote:
Show 12 quoted lines
> > From: Derrick Stolee <dstolee@microsoft.com> > > 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.
Show 60 quoted lines
>
> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
> ---
> 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)
> --
> gitgitgadgetMakes sense.