Re: [PATCH 07/11] pack-bitmap.c: avoid leaking via midx_bitmap_filename()

From: Taylor Blau <me@ttaylorr.com>
Date: Oct 22, 2021, 04:27 UTC
Message-ID: <YXI9zssVEupYAeWQ@nand.local>
In-Reply-To: <xmqqcznymi7k.fsf@gitster.g>

On Thu, Oct 21, 2021 at 09:54:39AM -0700, Junio C Hamano wrote:

Show 27 quoted lines

> But after reading all the users of get_midx_filename(), especially
> given that it is mostly internal implementation detail inside
> midx.c, I think rewriting
>
> 	f() {
> 	char *midx = get_midx_filename(...);
>
> 	... use midx ...
>
> 	cleanup:
> 	free(midx);
> 	}
>
> into
>
> 	f() {
> 	struct strbuf midx;
>
> 	get_midx_filename(&midx, ...);
>
> 	... use midx.buf ...
>
> 	cleanup:
> 	strbuf_release(&midx);
> 	}
>
> is not too bad a fix for this.

I was initially worried that this was too heavy-handed an approach. But I hadn't actually tried it, and updating the half-dozen callers of get_midx_filename() took only a couple of minutes.

But just because something is easy doesn't necessarily mean that it is a good idea to do ;). Luckily, this approach ended up being clean, and allows us to implement midx_bitmap_filename() without copying, open-coding, or leaking.

Thanks for the nice suggestion.

Taylor