Skip to content

Commit 1a6d8b9

Browse files
peffgitster
authored andcommitted
do not discard revindex when re-preparing packfiles
When an object lookup fails, we re-read the objects/pack directory to pick up any new packfiles that may have been created since our last read. We also discard any pack revindex structs we've allocated. The discarding is a problem for the pack-bitmap code, which keeps a pointer to the revindex for the bitmapped pack. After the discard, the pointer is invalid, and we may read free()d memory. Other revindex users do not keep a bare pointer to the revindex; instead, they always access it through revindex_for_pack(), which lazily builds the revindex. So one solution is to teach the pack-bitmap code a similar trick. It would be slightly less efficient, but probably not all that noticeable. However, it turns out this discarding is not actually necessary. When we call reprepare_packed_git, we do not throw away our old pack list. We keep the existing entries, and only add in new ones. So there is no safety problem; we will still have the pack struct that matches each revindex. The packfile itself may go away, of course, but we are already prepared to handle that, and it may happen outside of reprepare_packed_git anyway. Throwing away the revindex may save some RAM if the pack never gets reused (about 12 bytes per object). But it also wastes some CPU time (to regenerate the index) if the pack does get reused. It's hard to say which is more valuable, but in either case, it happens very rarely (only when we race with a simultaneous repack). Just leaving the revindex in place is simple and safe both for current and future code. Signed-off-by: Jeff King <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent ae4f07f commit 1a6d8b9

File tree

3 files changed

+0
-13
lines changed

3 files changed

+0
-13
lines changed

pack-revindex.c

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -245,14 +245,3 @@ struct revindex_entry *find_pack_revindex(struct packed_git *p, off_t ofs)
245245

246246
return pridx->revindex + pos;
247247
}
248-
249-
void discard_revindex(void)
250-
{
251-
if (pack_revindex_hashsz) {
252-
int i;
253-
for (i = 0; i < pack_revindex_hashsz; i++)
254-
free(pack_revindex[i].revindex);
255-
free(pack_revindex);
256-
pack_revindex_hashsz = 0;
257-
}
258-
}

pack-revindex.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,5 @@ struct pack_revindex *revindex_for_pack(struct packed_git *p);
1515
int find_revindex_position(struct pack_revindex *pridx, off_t ofs);
1616

1717
struct revindex_entry *find_pack_revindex(struct packed_git *p, off_t ofs);
18-
void discard_revindex(void);
1918

2019
#endif

sha1_file.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1279,7 +1279,6 @@ void prepare_packed_git(void)
12791279

12801280
void reprepare_packed_git(void)
12811281
{
1282-
discard_revindex();
12831282
prepare_packed_git_run_once = 0;
12841283
prepare_packed_git();
12851284
}

0 commit comments

Comments
 (0)