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

[PATCH 4/7] packed_object_info: hoist delta type resolution to helper

From
Jeff King <peff@peff.net>
Date
Jul 12, 2013, 06:31 UTC
Message-ID
<20130712063157.GD15572@sigill.intra.peff.net>
In-Reply-To
<20130712061533.GA11297@sigill.intra.peff.net>

To calculate the type of a packed object, we must walk down its delta chain until we hit a true base object with a real type. Most of the code in packed_object_info is for handling this case.

Let's hoist it out into a separate helper function, which will make it easier to make the type-lookup optional in the future (and keep our indentation level sane).

Signed-off-by: Jeff King <peff@peff.net>
---
Annoyingly, since the part we are moving comprises the bulk
of the function, the diff tends to show the opposite of what
actually happened: it looks like we renamed the function to its helper
name, and moved the other parts to a new function, which happens to have
the same name as the old one. So read the diff with care. :)
 sha1_file.c | 93 +++++++++++++++++++++++++++++++++++--------------------------
 1 file changed, 53 insertions(+), 40 deletions(-)
diff --git a/sha1_file.c b/sha1_file.c
index 39e7313..6f4aff3 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1713,52 +1713,21 @@ static int packed_object_info(struct packed_git *p, off_t obj_offset,
 	return type;
 }
 
-
 #define POI_STACK_PREALLOC 64
 
-static int packed_object_info(struct packed_git *p, off_t obj_offset,
-			      unsigned long *sizep, int *rtype,
-			      unsigned long *disk_sizep)
+static enum object_type packed_to_object_type(struct packed_git *p,
+					      off_t obj_offset,
+					      enum object_type type,
+					      struct pack_window **w_curs,
+					      off_t curpos)
 {
-	struct pack_window *w_curs = NULL;
-	unsigned long size;
-	off_t curpos = obj_offset;
-	enum object_type type;
 	off_t small_poi_stack[POI_STACK_PREALLOC];
 	off_t *poi_stack = small_poi_stack;
 	int poi_stack_nr = 0, poi_stack_alloc = POI_STACK_PREALLOC;
 
-	type = unpack_object_header(p, &w_curs, &curpos, &size);
-
-	if (rtype)
-		*rtype = type; /* representation type */
-
-	if (sizep) {
-		if (type == OBJ_OFS_DELTA || type == OBJ_REF_DELTA) {
-			off_t tmp_pos = curpos;
-			off_t base_offset = get_delta_base(p, &w_curs, &tmp_pos,
-							   type, obj_offset);
-			if (!base_offset) {
-				type = OBJ_BAD;
-				goto out;
-			}
-			*sizep = get_size_from_delta(p, &w_curs, tmp_pos);
-			if (*sizep == 0) {
-				type = OBJ_BAD;
-				goto out;
-			}
-		} else {
-			*sizep = size;
-		}
-	}
-
-	if (disk_sizep) {
-		struct revindex_entry *revidx = find_pack_revindex(p, obj_offset);
-		*disk_sizep = revidx[1].offset - obj_offset;
-	}
-
 	while (type == OBJ_OFS_DELTA || type == OBJ_REF_DELTA) {
 		off_t base_offset;
+		unsigned long size;
 		/* Push the object we're going to leave behind */
 		if (poi_stack_nr >= poi_stack_alloc && poi_stack == small_poi_stack) {
 			poi_stack_alloc = alloc_nr(poi_stack_nr);
@@ -1769,11 +1738,11 @@ static int packed_object_info(struct packed_git *p, off_t obj_offset,
 		}
 		poi_stack[poi_stack_nr++] = obj_offset;
 		/* If parsing the base offset fails, just unwind */
-		base_offset = get_delta_base(p, &w_curs, &curpos, type, obj_offset);
+		base_offset = get_delta_base(p, w_curs, &curpos, type, obj_offset);
 		if (!base_offset)
 			goto unwind;
 		curpos = obj_offset = base_offset;
-		type = unpack_object_header(p, &w_curs, &curpos, &size);
+		type = unpack_object_header(p, w_curs, &curpos, &size);
 		if (type <= OBJ_NONE) {
 			/* If getting the base itself fails, we first
 			 * retry the base, otherwise unwind */
@@ -1800,7 +1769,6 @@ out:
 out:
 	if (poi_stack != small_poi_stack)
 		free(poi_stack);
-	unuse_pack(&w_curs);
 	return type;
 
 unwind:
@@ -1814,6 +1782,51 @@ unwind:
 	goto out;
 }
 
+static int packed_object_info(struct packed_git *p, off_t obj_offset,
+			      unsigned long *sizep, int *rtype,
+			      unsigned long *disk_sizep)
+{
+	struct pack_window *w_curs = NULL;
+	unsigned long size;
+	off_t curpos = obj_offset;
+	enum object_type type;
+
+	type = unpack_object_header(p, &w_curs, &curpos, &size);
+
+	if (rtype)
+		*rtype = type; /* representation type */
+
+	if (sizep) {
+		if (type == OBJ_OFS_DELTA || type == OBJ_REF_DELTA) {
+			off_t tmp_pos = curpos;
+			off_t base_offset = get_delta_base(p, &w_curs, &tmp_pos,
+							   type, obj_offset);
+			if (!base_offset) {
+				type = OBJ_BAD;
+				goto out;
+			}
+			*sizep = get_size_from_delta(p, &w_curs, tmp_pos);
+			if (*sizep == 0) {
+				type = OBJ_BAD;
+				goto out;
+			}
+		} else {
+			*sizep = size;
+		}
+	}
+
+	if (disk_sizep) {
+		struct revindex_entry *revidx = find_pack_revindex(p, obj_offset);
+		*disk_sizep = revidx[1].offset - obj_offset;
+	}
+
+	type = packed_to_object_type(p, obj_offset, type, &w_curs, curpos);
+
+out:
+	unuse_pack(&w_curs);
+	return type;
+}
+
 static void *unpack_compressed_entry(struct packed_git *p,
 				    struct pack_window **w_curs,
 				    off_t curpos,
-- 
1.8.3.rc3.24.gec82cb9
Previous: Jeff KingNext: Jeff King
Message 11 of 16 in “cat-file --batch-check performance improvements”
  1. 0/7 cat-file --batch-check performance improvementsJeff King, Jul 12, 2013
  2. 1/7 cat-file: disable object/refname ambiguity check for batch modeJeff King, Jul 12, 2013
  3. Michael HaggertyJul 12, 2013
  4. Jeff KingJul 12, 2013
  5. Michael HaggertyJul 12, 2013
  6. Jeff KingJul 15, 2013
  7. Junio C HamanoJul 15, 2013
  8. Jeff KingJul 15, 2013
  9. 2/7 sha1_object_info_extended: rename "status" to "type"Jeff King, Jul 12, 2013
  10. 3/7 sha1_loose_object_info: make type lookup optionalJeff King, Jul 12, 2013
  11. 4/7 packed_object_info: hoist delta type resolution to helperJeff King, Jul 12, 2013
  12. 5/7 packed_object_info: make type lookup optionalJeff King, Jul 12, 2013
  13. 6/7 sha1_object_info_extended: make type calculation optionalJeff King, Jul 12, 2013
  14. 7/7 sha1_object_info_extended: pass object_info to helpersJeff King, Jul 12, 2013
  15. Junio C HamanoJul 12, 2013
  16. Jeff KingJul 12, 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.