Skip to content

Commit 47bf4b0

Browse files
peffgitster
authored andcommitted
prepare_packed_git_one: refactor duplicate-pack check
When we are reloading the list of packs, we check whether a particular pack has been loaded. This is slightly tricky, because we load packs based on the presence of their ".idx" files, but record the name of the matching ".pack" file. Therefore we want to compare their bases. The existing code stripped off ".idx" from a file we found, then compared that whole base length to strings containing the ".pack" version. This meant we could end up comparing bytes past what the ".pack" string contained, if the ".idx" file name was much longer. In practice, it worked OK because memcmp would end up seeing a difference in the two strings and would return early before hitting the full length. However, memcmp may sometimes read extra bytes past a difference (e.g., because it is comparing 64-bit words), or is even free to compare in reverse order. Furthermore, our memcmp made no guarantees that we matched the whole pack name, up to ".pack". So "foo.idx" would match "foo-bar.pack", which is wrong (but does not typically happen, because our pack names have a fixed size). We can fix both issues, avoid magic numbers, and document that we expect to compare against a string with ".pack" by using strip_suffix. Signed-off-by: Jeff King <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent d6cd00c commit 47bf4b0

File tree

1 file changed

+7
-2
lines changed

1 file changed

+7
-2
lines changed

sha1_file.c

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1197,17 +1197,22 @@ static void prepare_packed_git_one(char *objdir, int local)
11971197
dirnamelen = path.len;
11981198
while ((de = readdir(dir)) != NULL) {
11991199
struct packed_git *p;
1200+
size_t base_len;
12001201

12011202
if (is_dot_or_dotdot(de->d_name))
12021203
continue;
12031204

12041205
strbuf_setlen(&path, dirnamelen);
12051206
strbuf_addstr(&path, de->d_name);
12061207

1207-
if (ends_with(de->d_name, ".idx")) {
1208+
base_len = path.len;
1209+
if (strip_suffix_mem(path.buf, &base_len, ".idx")) {
12081210
/* Don't reopen a pack we already have. */
12091211
for (p = packed_git; p; p = p->next) {
1210-
if (!memcmp(path.buf, p->pack_name, path.len - 4))
1212+
size_t len;
1213+
if (strip_suffix(p->pack_name, ".pack", &len) &&
1214+
len == base_len &&
1215+
!memcmp(p->pack_name, path.buf, len))
12111216
break;
12121217
}
12131218
if (p == NULL &&

0 commit comments

Comments
 (0)