Re: [PATCH] merge-ort: only do pointer arithmetic for non-empty lists
- From
Andrzej Hunt <andrzej@ahunt.org>
- Date
- Apr 11, 2021, 09:14 UTC
- Message-ID
- <2cac1b8b-fe53-01c0-825c-0e10edb8a320@ahunt.org>
- In-Reply-To
- <xmqq8s5pkav2.fsf@gitster.g>
On 11/04/2021 00:56, Junio C Hamano wrote:
Show 19 quoted lines
> René Scharfe <l.s.r@web.de> writes: > > [...] >> Reading the diff I was wondering if QSORT now gets handed uninitialized >> values if version-nr is 0. The answer is no -- relevant_entries is >> initialized at declaration. Otherwise the compiler would have probably >> warned, but sometimes it gets confused. >> >> I wonder why relevant_entries is introduced at all, though. It's not >> referenced later. So how about this instead? >> >> if (versions->nr) >> QSORT(versions->items + offset, nr, tree_entry_order); >> >> The intent to sort the last versions->nr-offset entries of versions, >> but only if it's not empty, is easier to see like this, I think. > > That does make sense. I wonder if there needs some assertion to > ensure that "offset" does not exceed "versions.nr", though. >
I think so - I will add the assertion. Following the original offset calculations is not trivial- which seems like a good enough reason for the assert.