Subject: Re: [PATCH 09/11] pack-bitmap.c: more aggressively free in free_bitmap_index()

From: Junio C Hamano <[email-masked]>
Date: Thu, 21 Oct 2021 11:43:22 -0700

Taylor Blau <[email-masked]> writes:

> diff --git a/pack-bitmap.c b/pack-bitmap.c
> index 0f6656db02..451ca3512c 100644
> --- a/pack-bitmap.c
> +++ b/pack-bitmap.c
> @@ -1854,9 +1854,17 @@ void free_bitmap_index(struct bitmap_index *b)
>  	ewah_pool_free(b->trees);
>  	ewah_pool_free(b->blobs);
>  	ewah_pool_free(b->tags);
> +	if (b->bitmaps) {
> +		struct stored_bitmap *sb;
> +		kh_foreach_value(b->bitmaps, sb, {
> +			ewah_pool_free(sb->root);
> +			free(sb);
> +		});
> +	}
>  	kh_destroy_oid_map(b->bitmaps);
>  	free(b->ext_index.objects);
>  	free(b->ext_index.hashes);
> +	kh_destroy_oid_pos(b->ext_index.positions);
>  	bitmap_free(b->result);
>  	bitmap_free(b->haves);
>  	if (bitmap_is_midx(b)) {

It is not a fault of this patch, but I was reading the corresponding
load_bitmap() to see if we are covering everything here.

On the error path of that function, where it does "got failed", it
does not clean after itself very well, it seems.  If we read commits
bitmap successfully but failed to read trees bitmap, for example,
the resource held by commits bitmap is never reclaimed.