Skip to contentSkip to conversation

Don't die in git-http-fetch when fetching packs.

PATCH In review Shawn O. Pearce 2 participants 3 messages Jun 15, 2016

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Don't die in git-http-fetch when fetching packs.

My sp/mmap changes to pack-check.c modified the function such that it expects packed_git.pack_size to be populated with the total bytecount of the packfile by the caller.

But that isn't the case for packs obtained by git-http-fetch as pack_size was not initialized before being accessed. This caused verify_pack to think it had 2^32-21 bytes available when the downloaded pack perhaps was only 305 bytes in length. The use_pack function then later dies with "offset beyond end of packfile" when computing the overall file checksum.

---

For the tip of my sp/mmap topic. Without it HTTP fetch and push support is severely broken as it cannot deal with packs.

 http-fetch.c |    1 +
 http-push.c  |    1 +
 2 files changed, 2 insertions(+), 0 deletions(-)
http-fetch.c +1 -0
index fe8cd7b..67dfb0a 100644
@@ -809,6 +809,7 @@ static int fetch_pack(struct alt_base *repo, unsigned char *sha1)
 		return error("Unable to start request");
 	}
 
+	target->pack_size = ftell(packfile);
 	fclose(packfile);
 
 	ret = move_temp_to_file(tmpfile, filename);
http-push.c +1 -0
index 7e73eac..8eb448e 100644
@@ -770,6 +770,7 @@ static void finish_request(struct transfer_request *request)
 				request->url, curl_errorstr);
 			remote->can_update_info_refs = 0;
 		} else {
+			target->pack_size = ftell(request->local_stream);
 			fclose(request->local_stream);
 			request->local_stream = NULL;
 			if (!move_temp_to_file(request->tmpfile,
Signature · 1 line
1.4.4.4.gf198
#1 lore raw
Quoted 11 lines · Shawn O. Pearce

My sp/mmap changes to pack-check.c modified the function such that it expects packed_git.pack_size to be populated with the total bytecount of the packfile by the caller.

But that isn't the case for packs obtained by git-http-fetch as pack_size was not initialized before being accessed. This caused verify_pack to think it had 2^32-21 bytes available when the downloaded pack perhaps was only 305 bytes in length. The use_pack function then later dies with "offset beyond end of packfile" when computing the overall file checksum.

Quoted 5 lines

---

For the tip of my sp/mmap topic. Without it HTTP fetch and push support is severely broken as it cannot deal with packs.

Ah, thanks. I've seen this today and a repeated fetch after that fixed it and thought it was a mirroring lag.

#2 lore raw
Quoted 6 lines · Junio C Hamano
Quoted 2 lines · Shawn O. Pearce

For the tip of my sp/mmap topic. Without it HTTP fetch and push support is severely broken as it cannot deal with packs.

Ah, thanks. I've seen this today and a repeated fetch after that fixed it and thought it was a mirroring lag.

Me too. I have actually been fighting with it on and off since we got sp/mmap reasonably stable. I just always kept attributing it to kernel.org mirroring lag until I realized it was just happening way too often.

Signature · 1 line
Shawn.