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:32:16 -0700

Eric Sunshine <[email-masked]> writes:

> On Wed, Oct 20, 2021 at 11:40 PM Taylor Blau <[email-masked]> wrote:
>> The function free_bitmap_index() is somewhat lax in what it frees. There
>> are two notable examples:
>>
>>   - While it does call kh_destroy_oid_map on the "bitmaps" map (which
>>     maps) commit OIDs to their corresponding bitmaps, the bitmaps
>>     themselves are not freed. Note here that we recycle already-freed
>>     ewah_bitmaps into a pool, but these are handled correctly by
>>     ewah_pool_free().
>
> The parentheses placement seems off; it's not clear what the intent
> is. Perhaps either move the closing parenthesis to just before the
> comma or drop them altogether.

Yeah, I think we can do without them and the sentence becomes
clearer (we can add a comma before "which", too).

>>   - We never bother to free the extended index's "positions" map, which
>>     we always allocate in load_bitmap().
>>
>> Fix both of these.
>>
>> Signed-off-by: Taylor Blau <[email-masked]>