Re: [PATCH v3 2/9] cache-tree: simplify verify_cache() prototype
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jan 23, 2021, 21:10 UTC
- Message-ID
- <xmqqy2gj2wc0.fsf@gitster.c.googlers.com>
- In-Reply-To
- <CABPp-BEsoWs5ZEhS0KTHankzc8eUdmpn0uoF7t1ZtN8b2gwvBA@mail.gmail.com>
Elijah Newren <newren@gmail.com> writes:
Show 15 quoted lines
>> - 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];
>
> 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 think this is entirely my fault.
It probably reads more natural to start from 1 and interate through to the end of the array, comparing the current one with the previous entry, i.e.
for (i = 1; i < istate->cache_nr; i++) {
prev = cache[i - 1];
this = cache[i];
compare(prev, this);