Skip to content

Commit 8a38f46

Browse files
ttaylorrgitster
authored andcommitted
pack-bitmap: fix memory leak if load_bitmap() failed
After going through the "failed" label, load_bitmap() will return -1, and its caller (either prepare_bitmap_walk() or prepare_bitmap_git()) will then call free_bitmap_index(). That function would have done: struct stored_bitmap *sb; kh_foreach_value(b->bitmaps, sb { ewah_pool_free(sb->root); free(sb); }); , but won't since load_bitmap() already called kh_destroy_oid_map() and NULL'd the "bitmaps" pointer from within its "failed" label. So I think if you got part of the way through loading bitmap entries and then failed, you would leak all of the previous entries that you were able to load successfully. The solution is to remove the error handling code in load_bitmap(), because its caller will always call free_bitmap_index() in case of an error. Signed-off-by: Taylor Blau <[email protected]> Signed-off-by: Lidong Yan <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent b32feae commit 8a38f46

File tree

1 file changed

+4
-17
lines changed

1 file changed

+4
-17
lines changed

pack-bitmap.c

Lines changed: 4 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -630,41 +630,28 @@ static int load_bitmap(struct repository *r, struct bitmap_index *bitmap_git,
630630
bitmap_git->ext_index.positions = kh_init_oid_pos();
631631

632632
if (load_reverse_index(r, bitmap_git))
633-
goto failed;
633+
return -1;
634634

635635
if (!(bitmap_git->commits = read_bitmap_1(bitmap_git)) ||
636636
!(bitmap_git->trees = read_bitmap_1(bitmap_git)) ||
637637
!(bitmap_git->blobs = read_bitmap_1(bitmap_git)) ||
638638
!(bitmap_git->tags = read_bitmap_1(bitmap_git)))
639-
goto failed;
639+
return -1;
640640

641641
if (!bitmap_git->table_lookup && load_bitmap_entries_v1(bitmap_git) < 0)
642-
goto failed;
642+
return -1;
643643

644644
if (bitmap_git->base) {
645645
if (!bitmap_is_midx(bitmap_git))
646646
BUG("non-MIDX bitmap has non-NULL base bitmap index");
647647
if (load_bitmap(r, bitmap_git->base, 1) < 0)
648-
goto failed;
648+
return -1;
649649
}
650650

651651
if (!recursing)
652652
load_all_type_bitmaps(bitmap_git);
653653

654654
return 0;
655-
656-
failed:
657-
munmap(bitmap_git->map, bitmap_git->map_size);
658-
bitmap_git->map = NULL;
659-
bitmap_git->map_size = 0;
660-
661-
kh_destroy_oid_map(bitmap_git->bitmaps);
662-
bitmap_git->bitmaps = NULL;
663-
664-
kh_destroy_oid_pos(bitmap_git->ext_index.positions);
665-
bitmap_git->ext_index.positions = NULL;
666-
667-
return -1;
668655
}
669656

670657
static int open_pack_bitmap(struct repository *r,

0 commit comments

Comments
 (0)