Re: [PATCH] index-pack: eliminate unlimited recursion in get_delta_base()

From: Shawn Pearce <spearce@spearce.org>
Date: Dec 8, 2011, 03:02 UTC
Message-ID: <CAJo=hJvrk3Jzg3dQhQnfbmKAFovLuEtJAP4rakHPFeuZ0T5R7g@mail.gmail.com>
In-Reply-To: <1323280223-7990-1-git-send-email-pclouds@gmail.com>

2011/12/7 Nguyễn Thái Ngọc Duy <pclouds@gmail.com>:

Show 5 quoted lines

> Revert the order of delta applying so that by the time a delta is
> applied, its base is either non-delta or already inflated.
> get_delta_base() is still recursive, but because base's data is always
> ready, the inner get_delta_base() call never has any chance to call
> itself again.

...

Show 10 quoted lines

> @@ -508,10 +508,25 @@ static void *get_base_data(struct base_data *c)
>  {
>        if (!c->data) {
>                struct object_entry *obj = c->obj;
> +               struct base_data **delta = NULL;
> +               int delta_nr = 0, delta_alloc = 0;
>
> -               if (is_delta_type(obj->type)) {
> -                       void *base = get_base_data(c->base);
> -                       void *raw = get_data_from_pack(obj);

I think you missed the critical recursion. The real work is the recursion within find_unresolved_deltas(). This little helper get_base_data() shouldn't be tripping over these cases unless we have run out of delta_base_cache_limit and released objects near the base end of the delta chain, in which case this will restore them.

Maybe this is useful on its own, but in my opinion its not an interesting patch to consider without first fixing find_unresolved_deltas's recursion.