On Sat, Jan 23, 2021 at 1:02 PM Derrick Stolee <stolee@xxxxxxxxx> wrote: > > On 1/23/2021 3:24 PM, Elijah Newren wrote: > > On Sat, Jan 23, 2021 at 11:58 AM Derrick Stolee via GitGitGadget > > <gitgitgadget@xxxxxxxxx> wrote: > >> - for (i = 0; i < entries - 1; i++) { > >> + for (i = 0; i + 1 < istate->cache_nr; i++) { > >> /* path/file always comes after path because of the way > >> * the cache is sorted. Also path can appear only once, > >> * which means conflicting one would immediately follow. > >> */ > >> - const struct cache_entry *this_ce = cache[i]; > >> - const struct cache_entry *next_ce = cache[i + 1]; > >> + const struct cache_entry *this_ce = istate->cache[i]; > >> + const struct cache_entry *next_ce = istate->cache[i + 1]; > >> const char *this_name = this_ce->name; > >> const char *next_name = next_ce->name; > >> int this_len = ce_namelen(this_ce); > > Makes sense. Thanks for explaining the i + 1 < istate->cache_nr bit > > in the commit message; made it easier to read through quickly. I'm > > curious if it deserves a comment in the code too, since it does feel > > slightly unusual. > > I would argue that "i + 1 < N" is a more natural way to write this, > because we use "i + 1" as an index, so we want to ensure the index > we are about to use is within range. "i < N - 1" is the backwards > way to write that statement. Oh, right, I think I was reading too quickly and assuming one thing in my head (about what the code was going to do), and forgetting that assumption when I got to the actual code. Sorry about that; I agree with you, so ignore my previous comment.