Re: [PATCH v2 12/14] commit-graph: add large-filters bitmap chunk

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 5, 2020, 21:01 UTC
Message-ID: <xmqqbljoolhy.fsf@gitster.c.googlers.com>
In-Reply-To: <100b26d7c8a5e7ce21a950f33791eadab74e8e70.1596646576.git.me@ttaylorr.com>

Taylor Blau <me@ttaylorr.com> writes:

Show 8 quoted lines

> @@ -71,6 +72,10 @@ struct commit_graph {
>  	const unsigned char *chunk_base_graphs;
>  	const unsigned char *chunk_bloom_indexes;
>  	const unsigned char *chunk_bloom_data;
> +	const unsigned char *chunk_bloom_large_filters;
> +
> +	size_t bloom_large_to_alloc;
> +	struct bitmap bloom_large;

Hmph, is the API rich enough to allow users to release the resource used by such an embedded bitmap? I ask becuase...

Show 7 quoted lines

> @@ -2503,6 +2577,7 @@ void free_commit_graph(struct commit_graph *g)
>  	}
>  	free(g->filename);
>  	free(g->bloom_filter_settings);
> +	bitmap_free(g->bloom_large);
>  	free(g);
>  }

... this hunk cannot be possibly correct as-is, and cannot be made correct without changing g->bloom_large to a pointer into a heap allocated bitmap, because bitmap_free() wants to not just release the resource held by the bitmap but the bitmap itself.