diff mbox series

[08/11] pack-bitmap.c: don't leak type-level bitmaps

Message ID 29920e773527e6e82065205ed98262ce9b01de28.1634787555.git.me@ttaylorr.com (mailing list archive)
State Superseded
Headers show
Series midx: clean up t5319 under 'SANITIZE=leak' | expand

Commit Message

Taylor Blau Oct. 21, 2021, 3:40 a.m. UTC
test_bitmap_walk() is used to implement `git rev-list --test-bitmap`,
which compares the result of the on-disk bitmaps with ones generated
on-the-fly during a revision walk.

In fa95666a40 (pack-bitmap.c: harden 'test_bitmap_walk()' to check type
bitmaps, 2021-08-24), we hardened those tests to also check the four
special type-level bitmaps, but never freed those bitmaps. We should
have, since each required an allocation when we EWAH-decompressed them.

Free those, plugging that leak, and also free the base (the scratch-pad
bitmap), too.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
 pack-bitmap.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Junio C Hamano Oct. 21, 2021, 4:59 p.m. UTC | #1
Taylor Blau <me@ttaylorr.com> writes:

> test_bitmap_walk() is used to implement `git rev-list --test-bitmap`,
> which compares the result of the on-disk bitmaps with ones generated
> on-the-fly during a revision walk.
>
> In fa95666a40 (pack-bitmap.c: harden 'test_bitmap_walk()' to check type
> bitmaps, 2021-08-24), we hardened those tests to also check the four
> special type-level bitmaps, but never freed those bitmaps. We should
> have, since each required an allocation when we EWAH-decompressed them.
>
> Free those, plugging that leak, and also free the base (the scratch-pad
> bitmap), too.
>
> Signed-off-by: Taylor Blau <me@ttaylorr.com>
> ---

I've looked at the patches up to this point.  The ones I did not
comment (including this one ;-) looked perfect, and all of them
(including the ones I did comment) were cleanly described to easily
read the reasoning behind these changes, which I highly appreciated.

Especially if I did not quite agree with the reasoning, it helped me
clarify my thought where I found it not agreeable.

Thanks.
diff mbox series

Patch

diff --git a/pack-bitmap.c b/pack-bitmap.c
index d292e81af1..0f6656db02 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -1721,6 +1721,12 @@  void test_bitmap_walk(struct rev_info *revs)
 	else
 		die("mismatch in bitmap results");
 
+	bitmap_free(result);
+	bitmap_free(tdata.base);
+	bitmap_free(tdata.commits);
+	bitmap_free(tdata.trees);
+	bitmap_free(tdata.blobs);
+	bitmap_free(tdata.tags);
 	free_bitmap_index(bitmap_git);
 }