Merge pull request #5425 from lhchavez/fix-get-delta-base

pack: Improve error handling for get_delta_base()

3 files changed, 44 insertions, 26 deletions

diff --git a/src/indexer.c b/src/indexer.c
index 68fdd85c5..8dc6c7a7f 100644
--- a/src/indexer.c
+++ b/src/indexer.c
@@ -265,10 +265,11 @@ static int advance_delta_offset(git_indexer *idx, git_object_t type)
 	if (type == GIT_OBJECT_REF_DELTA) {
 		idx->off += GIT_OID_RAWSZ;
 	} else {
-		off64_t base_off = get_delta_base(idx->pack, &w, &idx->off, type, idx->entry_start);
+		off64_t base_off;
+		int error = get_delta_base(&base_off, idx->pack, &w, &idx->off, type, idx->entry_start);
 		git_mwindow_close(&w);
-		if (base_off < 0)
-			return (int)base_off;
+		if (error < 0)
+			return error;
 	}
 
 	return 0;
diff --git a/src/pack.c b/src/pack.c
index fcf64f57d..4294a6e32 100644
--- a/src/pack.c
+++ b/src/pack.c
@@ -488,8 +488,12 @@ int git_packfile_resolve_header(
 		size_t base_size;
 		git_packfile_stream stream;
 
-		base_offset = get_delta_base(p, &w_curs, &curpos, type, offset);
+		error = get_delta_base(&base_offset, p, &w_curs, &curpos, type, offset);
 		git_mwindow_close(&w_curs);
+
+		if (error < 0)
+			return error;
+
 		if ((error = git_packfile_stream_open(&stream, p, curpos)) < 0)
 			return error;
 		error = git_delta_read_header_fromstream(&base_size, size_p, &stream);
@@ -508,8 +512,12 @@ int git_packfile_resolve_header(
 			return error;
 		if (type != GIT_OBJECT_OFS_DELTA && type != GIT_OBJECT_REF_DELTA)
 			break;
-		base_offset = get_delta_base(p, &w_curs, &curpos, type, base_offset);
+
+		error = get_delta_base(&base_offset, p, &w_curs, &curpos, type, base_offset);
 		git_mwindow_close(&w_curs);
+
+		if (error < 0)
+			return error;
 	}
 	*type_p = type;
 
@@ -583,17 +591,11 @@ static int pack_dependency_chain(git_dependency_chain *chain_out,
 		if (type != GIT_OBJECT_OFS_DELTA && type != GIT_OBJECT_REF_DELTA)
 			break;
 
-		base_offset = get_delta_base(p, &w_curs, &curpos, type, obj_offset);
+		error = get_delta_base(&base_offset, p, &w_curs, &curpos, type, obj_offset);
 		git_mwindow_close(&w_curs);
 
-		if (base_offset == 0) {
-			error = packfile_error("delta offset is zero");
-			goto on_error;
-		}
-		if (base_offset < 0) { /* must actually be an error code */
-			error = (int)base_offset;
+		if (error < 0)
 			goto on_error;
-		}
 
 		/* we need to pass the pos *after* the delta-base bit */
 		elem->offset = curpos;
@@ -878,18 +880,21 @@ out:
  * curpos is where the data starts, delta_obj_offset is the where the
  * header starts
  */
-off64_t get_delta_base(
-	struct git_pack_file *p,
-	git_mwindow **w_curs,
-	off64_t *curpos,
-	git_object_t type,
-	off64_t delta_obj_offset)
+int get_delta_base(
+		off64_t *delta_base_out,
+		struct git_pack_file *p,
+		git_mwindow **w_curs,
+		off64_t *curpos,
+		git_object_t type,
+		off64_t delta_obj_offset)
 {
 	unsigned int left = 0;
 	unsigned char *base_info;
 	off64_t base_offset;
 	git_oid unused;
 
+	assert(delta_base_out);
+
 	base_info = pack_window_open(p, w_curs, *curpos, &left);
 	/* Assumption: the only reason this would fail is because the file is too small */
 	if (base_info == NULL)
@@ -909,12 +914,12 @@ off64_t get_delta_base(
 				return GIT_EBUFS;
 			unsigned_base_offset += 1;
 			if (!unsigned_base_offset || MSB(unsigned_base_offset, 7))
-				return 0; /* overflow */
+				return packfile_error("overflow");
 			c = base_info[used++];
 			unsigned_base_offset = (unsigned_base_offset << 7) + (c & 127);
 		}
 		if (unsigned_base_offset == 0 || (size_t)delta_obj_offset <= unsigned_base_offset)
-			return 0; /* out of bound */
+			return packfile_error("out of bounds");
 		base_offset = delta_obj_offset - unsigned_base_offset;
 		*curpos += used;
 	} else if (type == GIT_OBJECT_REF_DELTA) {
@@ -925,8 +930,12 @@ off64_t get_delta_base(
 
 			git_oid_fromraw(&oid, base_info);
 			if ((entry = git_oidmap_get(p->idx_cache, &oid)) != NULL) {
+				if (entry->offset == 0)
+					return packfile_error("delta offset is zero");
+
 				*curpos += 20;
-				return entry->offset;
+				*delta_base_out = entry->offset;
+				return 0;
 			} else {
 				/* If we're building an index, don't try to find the pack
 				 * entry; we just haven't seen it yet.  We'll make
@@ -941,9 +950,13 @@ off64_t get_delta_base(
 			return packfile_error("base entry delta is not in the same pack");
 		*curpos += 20;
 	} else
-		return 0;
+		return packfile_error("unknown object type");
 
-	return base_offset;
+	if (base_offset == 0)
+		return packfile_error("delta offset is zero");
+
+	*delta_base_out = base_offset;
+	return 0;
 }
 
 /***********************************************************
diff --git a/src/pack.h b/src/pack.h
index a6a32ff20..17ae72228 100644
--- a/src/pack.h
+++ b/src/pack.h
@@ -143,8 +143,12 @@ int git_packfile_stream_open(git_packfile_stream *obj, struct git_pack_file *p,
 ssize_t git_packfile_stream_read(git_packfile_stream *obj, void *buffer, size_t len);
 void git_packfile_stream_dispose(git_packfile_stream *obj);
 
-off64_t get_delta_base(struct git_pack_file *p, git_mwindow **w_curs,
-		off64_t *curpos, git_object_t type,
+int get_delta_base(
+		off64_t *delta_base_out,
+		struct git_pack_file *p,
+		git_mwindow **w_curs,
+		off64_t *curpos,
+		git_object_t type,
 		off64_t delta_obj_offset);
 
 void git_packfile_close(struct git_pack_file *p, bool unlink_packfile);