On Mon, Jun 28, 2021 at 7:05 PM Derrick Stolee via GitGitGadget
<gitgitgadget@xxxxxxxxx> wrote:
> - else
> +
> + /*
> + * Check for a sparse-directory entry named "path/".
> + * Due to the input p->path not having a trailing
> + * slash, the negative 'pos' value overshoots the
> + * expected position by at least one, hence "-2" here.
You added the qualifier "at least" to this comment since v5. I think
it's slightly misleading because it sounds like -2 is the end of the
special handling of the "at least" one overshoot. Perhaps if you
ended with... "hence '-2' instead of '-1' here, and we also need to
check below if we overshot more than one".
> + */
> + pos = -pos - 2;
> +
> + if (pos < 0 || pos >= o->src_index->cache_nr)
> return NULL;
> +
> + /*
> + * We might have multiple entries between 'pos' and
> + * the actual sparse-directory entry, so start walking
> + * back until finding it or passing where it would be.
It might be helpful to add a quick comment about the scenario where
this comes up. e.g.
This arises due to lexicographic sort ordering and sparse
directory entries coming with a trailing slash, causing there to be
multiple entries between "subdir" and "subdir/" (such as anything
beginning with "subdir." or "subdir-"). We are trying to walk back
from "subdir/" to "subdir" here.
> + */
> + while (pos >= 0) {
> + ce = o->src_index->cache[pos];
> +
> + if (strncmp(ce->name, p->path, p->pathlen))
> + return NULL;
> +
> + if (S_ISSPARSEDIR(ce->ce_mode) &&
> + sparse_dir_matches_path(ce, info, p))
> + return ce;
> +
> + pos--;
> + }
> +
> + return NULL;
> }
