git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 09/10] pack-revindex: use unsigned to store number of objects

From
Jeff King <peff@peff.net>
Date
Jul 10, 2013, 11:50 UTC
Message-ID
<20130710115026.GI21963@sigill.intra.peff.net>
In-Reply-To
<20130710113447.GA20113@sigill.intra.peff.net>

A packfile may have up to 2^32-1 objects in it, so the "right" data type to use is uint32_t. We currently use a signed int, which means that we may behave incorrectly for packfiles with more than 2^31-1 objects on 32-bit systems.

Nobody has noticed because having 2^31 objects is pretty insane. The linux.git repo has on the order of 2^22 objects, which is hundreds of times smaller than necessary to trigger the bug.

Let's bump this up to an "unsigned". On 32-bit systems, this gives us the correct data-type, and on 64-bit systems, it is probably more efficient to use the native "unsigned" than a true uint32_t.

While we're at it, we can fix the binary search not to overflow in such a case if our unsigned is 32 bits.

Signed-off-by: Jeff King <peff@peff.net>
---
I didn't look farther in the pack code to see if we have other
problematic instances. So there may be others lurking, but these ones
were close to the area I was working in.
 pack-revindex.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/pack-revindex.c b/pack-revindex.c
index 77a0465..1aa9754 100644
--- a/pack-revindex.c
+++ b/pack-revindex.c
@@ -72,8 +72,8 @@ static void create_pack_revindex(struct pack_revindex *rix)
 static void create_pack_revindex(struct pack_revindex *rix)
 {
 	struct packed_git *p = rix->p;
-	int num_ent = p->num_objects;
-	int i;
+	unsigned num_ent = p->num_objects;
+	unsigned i;
 	const char *index = p->index_data;
 
 	rix->revindex = xmalloc(sizeof(*rix->revindex) * (num_ent + 1));
@@ -114,7 +114,7 @@ struct revindex_entry *find_pack_revindex(struct packed_git *p, off_t ofs)
 struct revindex_entry *find_pack_revindex(struct packed_git *p, off_t ofs)
 {
 	int num;
-	int lo, hi;
+	unsigned lo, hi;
 	struct pack_revindex *rix;
 	struct revindex_entry *revindex;
 
@@ -132,7 +132,7 @@ struct revindex_entry *find_pack_revindex(struct packed_git *p, off_t ofs)
 	lo = 0;
 	hi = p->num_objects + 1;
 	do {
-		int mi = (lo + hi) / 2;
+		unsigned mi = lo + (hi - lo) / 2;
 		if (revindex[mi].offset == ofs) {
 			return revindex + mi;
 		} else if (ofs < revindex[mi].offset)
-- 
1.8.3.rc3.24.gec82cb9
Previous: Jeff KingNext: Jeff King
Message 44 of 52 in “cat-file --batch-disk-sizes”
  1. 0/4 cat-file --batch-disk-sizesJeff King, Jul 7, 2013
  2. 1/4 zero-initialize object_info structsJeff King, Jul 7, 2013
  3. Junio C HamanoJul 7, 2013
  4. 2/4 teach sha1_object_info_extended a "disk_size" queryJeff King, Jul 7, 2013
  5. 3/4 cat-file: add --batch-disk-sizes optionJeff King, Jul 7, 2013
  6. Junio C HamanoJul 7, 2013
  7. Jeff KingJul 7, 2013
  8. Duy NguyenJul 8, 2013
  9. Ramkumar RamachandraJul 8, 2013
  10. Duy NguyenJul 8, 2013
  11. Ramkumar RamachandraJul 8, 2013
  12. Duy NguyenJul 9, 2013
  13. Ramkumar RamachandraJul 9, 2013
  14. Jeff KingJul 10, 2013
  15. Junio C HamanoJul 8, 2013
  16. Jeff KingJul 10, 2013
  17. Junio C HamanoJul 11, 2013
  18. brian m. carlsonJul 7, 2013
  19. Jeff KingJul 10, 2013
  20. 4/4 pack-revindex: radix-sort the revindexJeff King, Jul 7, 2013
  21. Shawn PearceJul 7, 2013
  22. Jeff KingJul 8, 2013
  23. Shawn PearceJul 8, 2013
  24. Brandon CaseyJul 8, 2013
  25. Brandon CaseyJul 8, 2013
  26. Jeff KingJul 10, 2013
  27. Jeff KingJul 10, 2013
  28. 00/10 cat-file formats/on-disk sizesJeff King, Jul 10, 2013
  29. 01/10 zero-initialize object_info structsJeff King, Jul 10, 2013
  30. 02/10 teach sha1_object_info_extended a "disk_size" queryJeff King, Jul 10, 2013
  31. 03/10 t1006: modernize output comparisonsJeff King, Jul 10, 2013
  32. 04/10 cat-file: teach --batch to stream blob objectsJeff King, Jul 10, 2013
  33. 05/10 cat-file: refactor --batch option parsingJeff King, Jul 10, 2013
  34. 06/10 cat-file: add --batch-check=<format>Jeff King, Jul 10, 2013
  35. Eric SunshineJul 10, 2013
  36. Ramkumar RamachandraJul 10, 2013
  37. Jeff KingJul 11, 2013
  38. 07/10 cat-file: add %(objectsize:disk) format atomJeff King, Jul 10, 2013
  39. 08/10 cat-file: split --batch input lines on whitespaceJeff King, Jul 10, 2013
  40. Ramkumar RamachandraJul 10, 2013
  41. Jeff KingJul 11, 2013
  42. Junio C HamanoJul 11, 2013
  43. 08/10 cat-file: split --batch input lines on whitespaceJeff King, Jul 11, 2013
  44. 09/10 pack-revindex: use unsigned to store number of objectsJeff King, Jul 10, 2013
  45. 10/10 pack-revindex: radix-sort the revindexJeff King, Jul 10, 2013
  46. Jeff KingJul 10, 2013
  47. Ramkumar RamachandraJul 10, 2013
  48. Jeff KingJul 11, 2013
  49. Brandon CaseyJul 10, 2013
  50. Jeff KingJul 11, 2013
  51. 10/10 pack-revindex: radix-sort the revindexJeff King, Jul 11, 2013
  52. Brandon CaseyJul 11, 2013

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.