Subject: Re: [PATCH] index-pack: fix allocation of sorted_by_pos array

From: Junio C Hamano <[email-masked]>
Date: Mon, 06 Jul 2015 16:23:46 -0700

Junio C Hamano <[email-masked]> writes:

> When c6458e60 (index-pack: kill union delta_base to save memory,
> 2015-04-18) attempted to reduce the memory footprint of index-pack,
> one of the key thing it did was to keep track of ref-deltas and
> ofs-deltas separately.
>
> In fix_unresolved_deltas(), however it forgot that it now wants to
> look only at ref deltas in one place.  The code allocated an array
> for nr_unresolved, which is sum of number of ref- and ofs-deltas
> minus nr_resolved, which may be larger or smaller than the number
> ref-deltas.  Depending on nr_resolved, this was either under or over
> allocating.
>
> Also, the old code before this change had to use 'i' and 'n' because
> some of the things we see in the (old) deltas[] array we scanned
> with 'i' would not make it into the sorted_by_pos[] array in the old
> world order, but now because you have only ref delta in a separate
> ref_deltas[] array, they increment lock&step.  We no longer need
> separate variables.  And most importantly, we shouldn't pass the
> nr_unresolved parameter, as this number does not play a role in the
> working of this helper function.
>
> Helped-by: Nguyễn Thái Ngọc Duy <[email-masked]>
> Signed-off-by: Junio C Hamano <[email-masked]>
> ---
>
>  * This time, correcting the analysis; the previous one claimed that
>    this was not a bug but just an overallocation. It indeed is a bug
>    that uses an unrelated value that may or may not be sufficiently
>    large to hold the whole thing, I think.

I think this one as a real "crash hotfix" must be in the upcoming
release (the obvious alternative is to revert the series with
c6458e60 which I really want to avoid).

As Eric's "worktree add" would need some more time to solidify,
let's delay the -rc2 and later by a few weeks.