Merge pull request #659 from libgit2/development-merge

New-error-handling

1 files changed, 195 insertions, 160 deletions

diff --git a/src/pack.c b/src/pack.c
index 0d618324b..4a6bc6ae8 100644
--- a/src/pack.c
+++ b/src/pack.c
@@ -17,12 +17,12 @@
 #include <zlib.h>
 
 static int packfile_open(struct git_pack_file *p);
-static off_t nth_packed_object_offset(const struct git_pack_file *p, uint32_t n);
+static git_off_t nth_packed_object_offset(const struct git_pack_file *p, uint32_t n);
 int packfile_unpack_compressed(
 		git_rawobj *obj,
 		struct git_pack_file *p,
 		git_mwindow **w_curs,
-		off_t *curpos,
+		git_off_t *curpos,
 		size_t size,
 		git_otype type);
 
@@ -34,12 +34,18 @@ int packfile_unpack_compressed(
  * GIT_OID_MINPREFIXLEN and GIT_OID_HEXSZ.
  */
 static int pack_entry_find_offset(
-		off_t *offset_out,
+		git_off_t *offset_out,
 		git_oid *found_oid,
 		struct git_pack_file *p,
 		const git_oid *short_oid,
 		unsigned int len);
 
+static int packfile_error(const char *message)
+{
+	giterr_set(GITERR_ODB, "Invalid pack file - %s", message);
+	return -1;
+}
+
 /***********************************************************
  *
  * PACK INDEX METHODS
@@ -58,40 +64,31 @@ static int pack_index_check(const char *path, struct git_pack_file *p)
 {
 	struct git_pack_idx_header *hdr;
 	uint32_t version, nr, i, *index;
-
 	void *idx_map;
 	size_t idx_size;
-
 	struct stat st;
-
-	/* TODO: properly open the file without access time */
-	git_file fd = p_open(path, O_RDONLY /*| O_NOATIME */);
-
 	int error;
-
+	/* TODO: properly open the file without access time using O_NOATIME */
+	git_file fd = git_futils_open_ro(path);
 	if (fd < 0)
-		return git__throw(GIT_EOSERR, "Failed to check index. File missing or corrupted");
-
-	if (p_fstat(fd, &st) < GIT_SUCCESS) {
-		p_close(fd);
-		return git__throw(GIT_EOSERR, "Failed to check index. File appears to be corrupted");
-	}
-
-	if (!git__is_sizet(st.st_size))
-		return GIT_ENOMEM;
+		return fd;
 
-	idx_size = (size_t)st.st_size;
-
-	if (idx_size < 4 * 256 + 20 + 20) {
+	if (p_fstat(fd, &st) < 0 ||
+		!S_ISREG(st.st_mode) ||
+		!git__is_sizet(st.st_size) ||
+		(idx_size = (size_t)st.st_size) < 4 * 256 + 20 + 20)
+	{
 		p_close(fd);
-		return git__throw(GIT_EOBJCORRUPTED, "Failed to check index. Object is corrupted");
+		giterr_set(GITERR_OS, "Failed to check pack index.");
+		return -1;
 	}
 
 	error = git_futils_mmap_ro(&p->index_map, fd, 0, idx_size);
+
 	p_close(fd);
 
-	if (error < GIT_SUCCESS)
-		return git__rethrow(error, "Failed to check index");
+	if (error < 0)
+		return error;
 
 	hdr = idx_map = p->index_map.data;
 
@@ -100,7 +97,7 @@ static int pack_index_check(const char *path, struct git_pack_file *p)
 
 		if (version < 2 || version > 2) {
 			git_futils_mmap_free(&p->index_map);
-			return git__throw(GIT_EOBJCORRUPTED, "Failed to check index. Unsupported index version");
+			return packfile_error("unsupported index version");
 		}
 
 	} else
@@ -116,7 +113,7 @@ static int pack_index_check(const char *path, struct git_pack_file *p)
 		uint32_t n = ntohl(index[i]);
 		if (n < nr) {
 			git_futils_mmap_free(&p->index_map);
-			return git__throw(GIT_EOBJCORRUPTED, "Failed to check index. Index is non-monotonic");
+			return packfile_error("index is non-monotonic");
 		}
 		nr = n;
 	}
@@ -131,7 +128,7 @@ static int pack_index_check(const char *path, struct git_pack_file *p)
 		 */
 		if (idx_size != 4*256 + nr * 24 + 20 + 20) {
 			git_futils_mmap_free(&p->index_map);
-			return git__throw(GIT_EOBJCORRUPTED, "Failed to check index. Object is corrupted");
+			return packfile_error("index is corrupted");
 		}
 	} else if (version == 2) {
 		/*
@@ -155,39 +152,46 @@ static int pack_index_check(const char *path, struct git_pack_file *p)
 
 		if (idx_size < min_size || idx_size > max_size) {
 			git_futils_mmap_free(&p->index_map);
-			return git__throw(GIT_EOBJCORRUPTED, "Failed to check index. Wrong index size");
+			return packfile_error("wrong index size");
 		}
 	}
 
 	p->index_version = version;
 	p->num_objects = nr;
-	return GIT_SUCCESS;
+	return 0;
 }
 
 static int pack_index_open(struct git_pack_file *p)
 {
 	char *idx_name;
 	int error;
+	size_t name_len, offset;
 
 	if (p->index_map.data)
-		return GIT_SUCCESS;
+		return 0;
 
 	idx_name = git__strdup(p->pack_name);
-	strcpy(idx_name + strlen(idx_name) - strlen(".pack"), ".idx");
+	GITERR_CHECK_ALLOC(idx_name);
+
+	name_len = strlen(idx_name);
+	offset = name_len - strlen(".pack");
+	assert(offset < name_len); /* make sure no underflow */
+
+	strncpy(idx_name + offset, ".idx", name_len - offset);
 
 	error = pack_index_check(idx_name, p);
 	git__free(idx_name);
 
-	return error == GIT_SUCCESS ? GIT_SUCCESS : git__rethrow(error, "Failed to open index");
+	return error;
 }
 
 static unsigned char *pack_window_open(
 		struct git_pack_file *p,
 		git_mwindow **w_cursor,
-		off_t offset,
+		git_off_t offset,
 		unsigned int *left)
 {
-	if (p->mwf.fd == -1 && packfile_open(p) < GIT_SUCCESS)
+	if (p->mwf.fd == -1 && packfile_open(p) < 0)
 		return NULL;
 
 	/* Since packfiles end in a hash of their content and it's
@@ -201,7 +205,8 @@ static unsigned char *pack_window_open(
 	return git_mwindow_open(&p->mwf, w_cursor, offset, 20, left);
  }
 
-static unsigned long packfile_unpack_header1(
+static int packfile_unpack_header1(
+		unsigned long *usedp,
 		size_t *sizep,
 		git_otype *type,
 		const unsigned char *buf,
@@ -216,8 +221,13 @@ static unsigned long packfile_unpack_header1(
 	size = c & 15;
 	shift = 4;
 	while (c & 0x80) {
-		if (len <= used || bitsizeof(long) <= shift)
-			return 0;
+		if (len <= used)
+			return GIT_ESHORTBUFFER;
+
+		if (bitsizeof(long) <= shift) {
+			*usedp = 0;
+			return -1;
+		}
 
 		c = buf[used++];
 		size += (c & 0x7f) << shift;
@@ -225,7 +235,8 @@ static unsigned long packfile_unpack_header1(
 	}
 
 	*sizep = (size_t)size;
-	return used;
+	*usedp = used;
+	return 0;
 }
 
 int git_packfile_unpack_header(
@@ -233,11 +244,12 @@ int git_packfile_unpack_header(
 		git_otype *type_p,
 		git_mwindow_file *mwf,
 		git_mwindow **w_curs,
-		off_t *curpos)
+		git_off_t *curpos)
 {
 	unsigned char *base;
 	unsigned int left;
 	unsigned long used;
+	int ret;
 
 	/* pack_window_open() assures us we have [base, base + 20) available
 	 * as a range that we can look at at. (Its actually the hash
@@ -248,37 +260,39 @@ int git_packfile_unpack_header(
 //	base = pack_window_open(p, w_curs, *curpos, &left);
 	base = git_mwindow_open(mwf, w_curs, *curpos, 20, &left);
 	if (base == NULL)
-		return GIT_ENOMEM;
-
-	used = packfile_unpack_header1(size_p, type_p, base, left);
+		return GIT_ESHORTBUFFER;
 
-	if (used == 0)
-		return git__throw(GIT_EOBJCORRUPTED, "Header length is zero");
+	ret = packfile_unpack_header1(&used, size_p, type_p, base, left);
+	git_mwindow_close(w_curs);
+	if (ret == GIT_ESHORTBUFFER)
+		return ret;
+	else if (ret < 0)
+		return packfile_error("header length is zero");
 
 	*curpos += used;
-	return GIT_SUCCESS;
+	return 0;
 }
 
 static int packfile_unpack_delta(
 		git_rawobj *obj,
 		struct git_pack_file *p,
 		git_mwindow **w_curs,
-		off_t *curpos,
+		git_off_t *curpos,
 		size_t delta_size,
 		git_otype delta_type,
-		off_t obj_offset)
+		git_off_t obj_offset)
 {
-	off_t base_offset;
+	git_off_t base_offset;
 	git_rawobj base, delta;
 	int error;
 
 	base_offset = get_delta_base(p, w_curs, curpos, delta_type, obj_offset);
+	git_mwindow_close(w_curs);
 	if (base_offset == 0)
-		return git__throw(GIT_EOBJCORRUPTED, "Delta offset is zero");
-	if (base_offset < 0)
-		return git__rethrow(base_offset, "Failed to get delta base");
+		return packfile_error("delta offset is zero");
+	if (base_offset < 0) /* must actually be an error code */
+		return (int)base_offset;
 
-	git_mwindow_close(w_curs);
 	error = git_packfile_unpack(&base, p, &base_offset);
 
 	/*
@@ -287,35 +301,35 @@ static int packfile_unpack_delta(
 	 *
 	 * We'll need to do this in order to support thin packs.
 	 */
-	if (error < GIT_SUCCESS)
-		return git__rethrow(error, "Corrupted delta");
+	if (error < 0)
+		return error;
 
 	error = packfile_unpack_compressed(&delta, p, w_curs, curpos, delta_size, delta_type);
-	if (error < GIT_SUCCESS) {
+	git_mwindow_close(w_curs);
+	if (error < 0) {
 		git__free(base.data);
-		return git__rethrow(error, "Corrupted delta");
+		return error;
 	}
 
 	obj->type = base.type;
-	error = git__delta_apply(obj,
-			base.data, base.len,
-			delta.data, delta.len);
+	error = git__delta_apply(obj, base.data, base.len, delta.data, delta.len);
 
 	git__free(base.data);
 	git__free(delta.data);
 
 	/* TODO: we might want to cache this shit. eventually */
 	//add_delta_base_cache(p, base_offset, base, base_size, *type);
+
 	return error; /* error set by git__delta_apply */
 }
 
 int git_packfile_unpack(
-		git_rawobj *obj,
-		struct git_pack_file *p,
-		off_t *obj_offset)
+	git_rawobj *obj,
+	struct git_pack_file *p,
+	git_off_t *obj_offset)
 {
 	git_mwindow *w_curs = NULL;
-	off_t curpos = *obj_offset;
+	git_off_t curpos = *obj_offset;
 	int error;
 
 	size_t size = 0;
@@ -330,8 +344,10 @@ int git_packfile_unpack(
 	obj->type = GIT_OBJ_BAD;
 
 	error = git_packfile_unpack_header(&size, &type, &p->mwf, &w_curs, &curpos);
-	if (error < GIT_SUCCESS)
-		return git__rethrow(error, "Failed to unpack packfile");
+	git_mwindow_close(&w_curs);
+
+	if (error < 0)
+		return error;
 
 	switch (type) {
 	case GIT_OBJ_OFS_DELTA:
@@ -351,52 +367,70 @@ int git_packfile_unpack(
 		break;
 
 	default:
-		error = GIT_EOBJCORRUPTED;
+		error = packfile_error("invalid packfile type in header");;
 		break;
 	}
 
-	git_mwindow_close(&w_curs);
+	*obj_offset = curpos;
+	return error;
+}
 
-	if (error < GIT_SUCCESS)
-		return git__rethrow(error, "Failed to unpack object");
+static void *use_git_alloc(void *opaq, unsigned int count, unsigned int size)
+{
+	GIT_UNUSED(opaq);
+	return git__calloc(count, size);
+}
 
-	*obj_offset = curpos;
-	return GIT_SUCCESS;
+static void use_git_free(void *opaq, void *ptr)
+{
+	GIT_UNUSED(opaq);
+	git__free(ptr);
 }
 
 int packfile_unpack_compressed(
-		git_rawobj *obj,
-		struct git_pack_file *p,
-		git_mwindow **w_curs,
-		off_t *curpos,
-		size_t size,
-		git_otype type)
+	git_rawobj *obj,
+	struct git_pack_file *p,
+	git_mwindow **w_curs,
+	git_off_t *curpos,
+	size_t size,
+	git_otype type)
 {
 	int st;
 	z_stream stream;
 	unsigned char *buffer, *in;
 
-	buffer = git__malloc(size + 1);
-	memset(buffer, 0x0, size + 1);
+	buffer = git__calloc(1, size + 1);
+	GITERR_CHECK_ALLOC(buffer);
 
 	memset(&stream, 0, sizeof(stream));
 	stream.next_out = buffer;
 	stream.avail_out = (uInt)size + 1;
+	stream.zalloc = use_git_alloc;
+	stream.zfree = use_git_free;
 
 	st = inflateInit(&stream);
 	if (st != Z_OK) {
 		git__free(buffer);
-		return git__throw(GIT_EZLIB, "Error in zlib");
+		giterr_set(GITERR_ZLIB, "Failed to inflate packfile");
+
+		return -1;
 	}
 
 	do {
 		in = pack_window_open(p, w_curs, *curpos, &stream.avail_in);
 		stream.next_in = in;
 		st = inflate(&stream, Z_FINISH);
+		git_mwindow_close(w_curs);
 
 		if (!stream.avail_out)
 			break; /* the payload is larger than it should be */
 
+		if (st == Z_BUF_ERROR && in == NULL) {
+			inflateEnd(&stream);
+			git__free(buffer);
+			return GIT_ESHORTBUFFER;
+		}
+
 		*curpos += stream.next_in - in;
 	} while (st == Z_OK || st == Z_BUF_ERROR);
 
@@ -404,30 +438,36 @@ int packfile_unpack_compressed(
 
 	if ((st != Z_STREAM_END) || stream.total_out != size) {
 		git__free(buffer);
-		return git__throw(GIT_EZLIB, "Error in zlib");
+		giterr_set(GITERR_ZLIB, "Failed to inflate packfile");
+		return -1;
 	}
 
 	obj->type = type;
 	obj->len = size;
 	obj->data = buffer;
-	return GIT_SUCCESS;
+	return 0;
 }
 
 /*
  * curpos is where the data starts, delta_obj_offset is the where the
  * header starts
  */
-off_t get_delta_base(
-		struct git_pack_file *p,
-		git_mwindow **w_curs,
-		off_t *curpos,
-		git_otype type,
-		off_t delta_obj_offset)
+git_off_t get_delta_base(
+	struct git_pack_file *p,
+	git_mwindow **w_curs,
+	git_off_t *curpos,
+	git_otype type,
+	git_off_t delta_obj_offset)
 {
-	unsigned char *base_info = pack_window_open(p, w_curs, *curpos, NULL);
-	off_t base_offset;
+	unsigned int left = 0;
+	unsigned char *base_info;
+	git_off_t base_offset;
 	git_oid unused;
 
+	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)
+		return GIT_ESHORTBUFFER;
 	/* pack_window_open() assured us we have [base_info, base_info + 20)
 	 * as a range that we can look at without walking off the
 	 * end of the mapped window. Its actually the hash size
@@ -439,6 +479,8 @@ off_t get_delta_base(
 		unsigned char c = base_info[used++];
 		base_offset = c & 127;
 		while (c & 128) {
+			if (left <= used)
+				return GIT_ESHORTBUFFER;
 			base_offset += 1;
 			if (!base_offset || MSB(base_offset, 7))
 				return 0; /* overflow */
@@ -463,8 +505,8 @@ off_t get_delta_base(
 			}
 		}
 		/* The base entry _must_ be in the same pack */
-		if (pack_entry_find_offset(&base_offset, &unused, p, (git_oid *)base_info, GIT_OID_HEXSZ) < GIT_SUCCESS)
-			return git__rethrow(GIT_EPACKCORRUPTED, "Base entry delta is not in the same pack");
+		if (pack_entry_find_offset(&base_offset, &unused, p, (git_oid *)base_info, GIT_OID_HEXSZ) < 0)
+			return packfile_error("base entry delta is not in the same pack");
 		*curpos += 20;
 	} else
 		return 0;
@@ -478,11 +520,11 @@ off_t get_delta_base(
  *
  ***********************************************************/
 
-static struct git_pack_file *packfile_alloc(int extra)
+static struct git_pack_file *packfile_alloc(size_t extra)
 {
-	struct git_pack_file *p = git__malloc(sizeof(*p) + extra);
-	memset(p, 0, sizeof(*p));
-	p->mwf.fd = -1;
+	struct git_pack_file *p = git__calloc(1, sizeof(*p) + extra);
+	if (p != NULL)
+		p->mwf.fd = -1;
 	return p;
 }
 
@@ -510,24 +552,25 @@ static int packfile_open(struct git_pack_file *p)
 	git_oid sha1;
 	unsigned char *idx_sha1;
 
-	if (!p->index_map.data && pack_index_open(p) < GIT_SUCCESS)
-		return git__throw(GIT_ENOTFOUND, "Failed to open packfile. File not found");
+	assert(p->index_map.data);
+
+	if (!p->index_map.data && pack_index_open(p) < 0)
+		return git_odb__error_notfound("failed to open packfile", NULL);
 
 	/* TODO: open with noatime */
-	p->mwf.fd = p_open(p->pack_name, O_RDONLY);
-	if (p->mwf.fd < 0 || p_fstat(p->mwf.fd, &st) < GIT_SUCCESS)
-		return git__throw(GIT_EOSERR, "Failed to open packfile. File appears to be corrupted");
+	p->mwf.fd = git_futils_open_ro(p->pack_name);
+	if (p->mwf.fd < 0)
+		return p->mwf.fd;
 
-	if (git_mwindow_file_register(&p->mwf) < GIT_SUCCESS) {
-		p_close(p->mwf.fd);
-		return git__throw(GIT_ERROR, "Failed to register packfile windows");
-	}
+	if (p_fstat(p->mwf.fd, &st) < 0 ||
+		git_mwindow_file_register(&p->mwf) < 0)
+		goto cleanup;
 
 	/* If we created the struct before we had the pack we lack size. */
 	if (!p->mwf.size) {
 		if (!S_ISREG(st.st_mode))
 			goto cleanup;
-		p->mwf.size = (off_t)st.st_size;
+		p->mwf.size = (git_off_t)st.st_size;
 	} else if (p->mwf.size != st.st_size)
 		goto cleanup;
 
@@ -537,44 +580,35 @@ static int packfile_open(struct git_pack_file *p)
 	 */
 	fd_flag = fcntl(p->mwf.fd, F_GETFD, 0);
 	if (fd_flag < 0)
-		return error("cannot determine file descriptor flags");
+		goto cleanup;
 
 	fd_flag |= FD_CLOEXEC;
 	if (fcntl(p->pack_fd, F_SETFD, fd_flag) == -1)
-		return GIT_EOSERR;
+		goto cleanup;
 #endif
 
 	/* Verify we recognize this pack file format. */
-	if (p_read(p->mwf.fd, &hdr, sizeof(hdr)) < GIT_SUCCESS)
-		goto cleanup;
-
-	if (hdr.hdr_signature != htonl(PACK_SIGNATURE))
-		goto cleanup;
-
-	if (!pack_version_ok(hdr.hdr_version))
+	if (p_read(p->mwf.fd, &hdr, sizeof(hdr)) < 0 ||
+		hdr.hdr_signature != htonl(PACK_SIGNATURE) ||
+		!pack_version_ok(hdr.hdr_version))
 		goto cleanup;
 
 	/* Verify the pack matches its index. */
-	if (p->num_objects != ntohl(hdr.hdr_entries))
-		goto cleanup;
-
-	if (p_lseek(p->mwf.fd, p->mwf.size - GIT_OID_RAWSZ, SEEK_SET) == -1)
-		goto cleanup;
-
-	if (p_read(p->mwf.fd, sha1.id, GIT_OID_RAWSZ) < GIT_SUCCESS)
+	if (p->num_objects != ntohl(hdr.hdr_entries) ||
+		p_lseek(p->mwf.fd, p->mwf.size - GIT_OID_RAWSZ, SEEK_SET) == -1 ||
+		p_read(p->mwf.fd, sha1.id, GIT_OID_RAWSZ) < 0)
 		goto cleanup;
 
 	idx_sha1 = ((unsigned char *)p->index_map.data) + p->index_map.len - 40;
 
-	if (git_oid_cmp(&sha1, (git_oid *)idx_sha1) != 0)
-		goto cleanup;
-
-	return GIT_SUCCESS;
+	if (git_oid_cmp(&sha1, (git_oid *)idx_sha1) == 0)
+		return 0;
 
 cleanup:
+	giterr_set(GITERR_OS, "Invalid packfile '%s'", p->pack_name);
 	p_close(p->mwf.fd);
 	p->mwf.fd = -1;
-	return git__throw(GIT_EPACKCORRUPTED, "Failed to open packfile. Pack is corrupted");
+	return -1;
 }
 
 int git_packfile_check(struct git_pack_file **pack_out, const char *path)
@@ -586,6 +620,7 @@ int git_packfile_check(struct git_pack_file **pack_out, const char *path)
 	*pack_out = NULL;
 	path_len = strlen(path);
 	p = packfile_alloc(path_len + 2);
+	GITERR_CHECK_ALLOC(p);
 
 	/*
 	 * Make sure a corresponding .pack file exists and that
@@ -594,19 +629,19 @@ int git_packfile_check(struct git_pack_file **pack_out, const char *path)
 	path_len -= strlen(".idx");
 	if (path_len < 1) {
 		git__free(p);
-		return git__throw(GIT_ENOTFOUND, "Failed to check packfile. Wrong path name");
+		return git_odb__error_notfound("invalid packfile path", NULL);
 	}
 
 	memcpy(p->pack_name, path, path_len);
 
 	strcpy(p->pack_name + path_len, ".keep");
-	if (git_path_exists(p->pack_name) == GIT_SUCCESS)
+	if (git_path_exists(p->pack_name) == true)
 		p->pack_keep = 1;
 
 	strcpy(p->pack_name + path_len, ".pack");
-	if (p_stat(p->pack_name, &st) < GIT_SUCCESS || !S_ISREG(st.st_mode)) {
+	if (p_stat(p->pack_name, &st) < 0 || !S_ISREG(st.st_mode)) {
 		git__free(p);
-		return git__throw(GIT_ENOTFOUND, "Failed to check packfile. File not found");
+		return git_odb__error_notfound("packfile not found", NULL);
 	}
 
 	/* ok, it looks sane as far as we can check without
@@ -618,11 +653,12 @@ int git_packfile_check(struct git_pack_file **pack_out, const char *path)
 
 	/* see if we can parse the sha1 oid in the packfile name */
 	if (path_len < 40 ||
-		git_oid_fromstr(&p->sha1, path + path_len - GIT_OID_HEXSZ) < GIT_SUCCESS)
+		git_oid_fromstr(&p->sha1, path + path_len - GIT_OID_HEXSZ) < 0)
 		memset(&p->sha1, 0x0, GIT_OID_RAWSZ);
 
 	*pack_out = p;
-	return GIT_SUCCESS;
+
+	return 0;
 }
 
 /***********************************************************
@@ -631,7 +667,7 @@ int git_packfile_check(struct git_pack_file **pack_out, const char *path)
  *
  ***********************************************************/
 
-static off_t nth_packed_object_offset(const struct git_pack_file *p, uint32_t n)
+static git_off_t nth_packed_object_offset(const struct git_pack_file *p, uint32_t n)
 {
 	const unsigned char *index = p->index_map.data;
 	index += 4 * 256;
@@ -650,11 +686,11 @@ static off_t nth_packed_object_offset(const struct git_pack_file *p, uint32_t n)
 }
 
 static int pack_entry_find_offset(
-		off_t *offset_out,
-		git_oid *found_oid,
-		struct git_pack_file *p,
-		const git_oid *short_oid,
-		unsigned int len)
+	git_off_t *offset_out,
+	git_oid *found_oid,
+	struct git_pack_file *p,
+	const git_oid *short_oid,
+	unsigned int len)
 {
 	const uint32_t *level1_ofs = p->index_map.data;
 	const unsigned char *index = p->index_map.data;
@@ -667,8 +703,8 @@ static int pack_entry_find_offset(
 	if (index == NULL) {
 		int error;
 
-		if ((error = pack_index_open(p)) < GIT_SUCCESS)
-			return git__rethrow(error, "Failed to find offset for pack entry");
+		if ((error = pack_index_open(p)) < 0)
+			return error;
 
 		assert(p->index_map.data);
 
@@ -711,9 +747,8 @@ static int pack_entry_find_offset(
 		if (pos < (int)p->num_objects) {
 			current = index + pos * stride;
 
-			if (!git_oid_ncmp(short_oid, (const git_oid *)current, len)) {
+			if (!git_oid_ncmp(short_oid, (const git_oid *)current, len))
 				found = 1;
-			}
 		}
 	}
 
@@ -726,22 +761,22 @@ static int pack_entry_find_offset(
 		}
 	}
 
-	if (!found) {
-		return git__throw(GIT_ENOTFOUND, "Failed to find offset for pack entry. Entry not found");
-	} else if (found > 1) {
-		return git__throw(GIT_EAMBIGUOUSOIDPREFIX, "Failed to find offset for pack entry. Ambiguous sha1 prefix within pack");
-	} else {
-		*offset_out = nth_packed_object_offset(p, pos);
-		git_oid_fromraw(found_oid, current);
+	if (!found)
+		return git_odb__error_notfound("failed to find offset for pack entry", short_oid);
+	if (found > 1)
+		return git_odb__error_ambiguous("found multiple offsets for pack entry");
+	*offset_out = nth_packed_object_offset(p, pos);
+	git_oid_fromraw(found_oid, current);
 
 #ifdef INDEX_DEBUG_LOOKUP
+	{
 		unsigned char hex_sha1[GIT_OID_HEXSZ + 1];
 		git_oid_fmt(hex_sha1, found_oid);
 		hex_sha1[GIT_OID_HEXSZ] = '\0';
 		printf("found lo=%d %s\n", lo, hex_sha1);
-#endif
-		return GIT_SUCCESS;
 	}
+#endif
+	return 0;
 }
 
 int git_pack_entry_find(
@@ -750,7 +785,7 @@ int git_pack_entry_find(
 		const git_oid *short_oid,
 		unsigned int len)
 {
-	off_t offset;
+	git_off_t offset;
 	git_oid found_oid;
 	int error;
 
@@ -760,22 +795,22 @@ int git_pack_entry_find(
 		unsigned i;
 		for (i = 0; i < p->num_bad_objects; i++)
 			if (git_oid_cmp(short_oid, &p->bad_object_sha1[i]) == 0)
-				return git__throw(GIT_ERROR, "Failed to find pack entry. Bad object found");
+				return packfile_error("bad object found in packfile");
 	}
 
 	error = pack_entry_find_offset(&offset, &found_oid, p, short_oid, len);
-	if (error < GIT_SUCCESS)
-		return git__rethrow(error, "Failed to find pack entry. Couldn't find offset");
+	if (error < 0)
+		return error;
 
 	/* we found a unique entry in the index;
 	 * make sure the packfile backing the index
 	 * still exists on disk */
-	if (p->mwf.fd == -1 && packfile_open(p) < GIT_SUCCESS)
-		return git__throw(GIT_EOSERR, "Failed to find pack entry. Packfile doesn't exist on disk");
+	if (p->mwf.fd == -1 && (error = packfile_open(p)) < 0)
+		return error;
 
 	e->offset = offset;
 	e->p = p;
 
 	git_oid_cpy(&e->sha1, &found_oid);
-	return GIT_SUCCESS;
+	return 0;
 }