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

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 21, 2021, 18:43 UTC
Message-ID: <xmqqlf2mkylx.fsf@gitster.g>
In-Reply-To: <e65ac7deb5973b8209e8d94ad9140e005e22c44e.1634787555.git.me@ttaylorr.com>

Taylor Blau <me@ttaylorr.com> writes:

Show 22 quoted lines

> 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.