Merge pull request #659 from libgit2/development-merge

New-error-handling

1 files changed, 147 insertions, 171 deletions

diff --git a/src/odb.c b/src/odb.c
index 81fc82ba8..934b317ed 100644
--- a/src/odb.c
+++ b/src/odb.c
@@ -32,10 +32,7 @@ static int format_object_header(char *hdr, size_t n, size_t obj_len, git_otype o
 {
 	const char *type_str = git_object_type2string(obj_type);
 	int len = p_snprintf(hdr, n, "%s %"PRIuZ, type_str, obj_len);
-
-	if (len < 0 || len >= (int)n)
-		return git__throw(GIT_ERROR, "Cannot format object header. Length is out of bounds");
-
+	assert(len > 0 && len <= (int)n);
 	return len+1;
 }
 
@@ -48,13 +45,11 @@ int git_odb__hashobj(git_oid *id, git_rawobj *obj)
 	assert(id && obj);
 
 	if (!git_object_typeisloose(obj->type))
-		return git__throw(GIT_ERROR, "Failed to hash object. Wrong object type");
-
+		return -1;
 	if (!obj->data && obj->len != 0)
-		return git__throw(GIT_ERROR, "Failed to hash object. No data given");
+		return -1;
 
-	if ((hdrlen = format_object_header(header, sizeof(header), obj->len, obj->type)) < 0)
-		return git__rethrow(hdrlen, "Failed to hash object");
+	hdrlen = format_object_header(header, sizeof(header), obj->len, obj->type);
 
 	vec[0].data = header;
 	vec[0].len = hdrlen;
@@ -63,7 +58,7 @@ int git_odb__hashobj(git_oid *id, git_rawobj *obj)
 
 	git_hash_vec(id, vec, 2);
 
-	return GIT_SUCCESS;
+	return 0;
 }
 
 
@@ -120,8 +115,6 @@ int git_odb__hashfd(git_oid *out, git_file fd, size_t size, git_otype type)
 	git_hash_ctx *ctx;
 
 	hdr_len = format_object_header(hdr, sizeof(hdr), size, type);
-	if (hdr_len < 0)
-		return git__throw(GIT_ERROR, "Failed to format blob header. Length is out of bounds");
 
 	ctx = git_hash_new_ctx();
 
@@ -132,7 +125,8 @@ int git_odb__hashfd(git_oid *out, git_file fd, size_t size, git_otype type)
 
 		if (read_len < 0) {
 			git_hash_free_ctx(ctx);
-			return git__throw(GIT_EOSERR, "Error when reading file: %s", strerror(errno));
+			giterr_set(GITERR_OS, "Error reading file");
+			return -1;
 		}
 
 		git_hash_update(ctx, buffer, read_len);
@@ -142,69 +136,67 @@ int git_odb__hashfd(git_oid *out, git_file fd, size_t size, git_otype type)
 	git_hash_final(out, ctx);
 	git_hash_free_ctx(ctx);
 
-	return GIT_SUCCESS;
+	return 0;
 }
 
 int git_odb__hashlink(git_oid *out, const char *path)
 {
 	struct stat st;
-	int error;
 	git_off_t size;
+	int result;
 
-	error = p_lstat(path, &st);
-	if (error < 0)
-		return git__throw(GIT_EOSERR, "Failed to stat blob. %s", strerror(errno));
+	if (git_path_lstat(path, &st) < 0)
+		return -1;
 
 	size = st.st_size;
 
-	if (!git__is_sizet(size))
-		return git__throw(GIT_EOSERR, "File size overflow for 32-bit systems");
+	if (!git__is_sizet(size)) {
+		giterr_set(GITERR_OS, "File size overflow for 32-bit systems");
+		return -1;
+	}
 
 	if (S_ISLNK(st.st_mode)) {
 		char *link_data;
 		ssize_t read_len;
 
 		link_data = git__malloc((size_t)size);
-		if (link_data == NULL)
-			return GIT_ENOMEM;
+		GITERR_CHECK_ALLOC(link_data);
 
 		read_len = p_readlink(path, link_data, (size_t)(size + 1));
-		if (read_len != (ssize_t)size)
-			return git__throw(GIT_EOSERR, "Failed to read symlink data");
+		if (read_len != (ssize_t)size) {
+			giterr_set(GITERR_OS, "Failed to read symlink data for '%s'", path);
+			return -1;
+		}
 
-		error = git_odb_hash(out, link_data, (size_t)size, GIT_OBJ_BLOB);
-		free(link_data);
+		result = git_odb_hash(out, link_data, (size_t)size, GIT_OBJ_BLOB);
+		git__free(link_data);
 	} else { 
-		int fd;
-
-		if ((fd = p_open(path, O_RDONLY)) < 0)
-			return git__throw(GIT_ENOTFOUND, "Could not open '%s'", path);
-
-		error = git_odb__hashfd(out, fd, (size_t)size, GIT_OBJ_BLOB);
+		int fd = git_futils_open_ro(path);
+		if (fd < 0)
+			return -1;
+		result = git_odb__hashfd(out, fd, (size_t)size, GIT_OBJ_BLOB);
 		p_close(fd);
 	}
 
-	return error;
+	return result;
 }
 
 int git_odb_hashfile(git_oid *out, const char *path, git_otype type)
 {
-	int fd, error;
 	git_off_t size;
-
-	if ((fd = p_open(path, O_RDONLY)) < 0)
-		return git__throw(GIT_ENOTFOUND, "Could not open '%s'", path);
+	int result, fd = git_futils_open_ro(path);
+	if (fd < 0)
+		return fd;
 
 	if ((size = git_futils_filesize(fd)) < 0 || !git__is_sizet(size)) {
+		giterr_set(GITERR_OS, "File size overflow for 32-bit systems");
 		p_close(fd);
-		return git__throw(GIT_EOSERR,
-			"File size overflow. The object is too big to fit in 32-bit mode");
+		return -1;
 	}
 
-	error = git_odb__hashfd(out, fd, (size_t)size, type);
-
+	result = git_odb__hashfd(out, fd, (size_t)size, type);
 	p_close(fd);
-	return error;
+	return result;
 }
 
 int git_odb_hash(git_oid *id, const void *data, size_t len, git_otype type)
@@ -242,11 +234,11 @@ static int fake_wstream__write(git_odb_stream *_stream, const char *data, size_t
 	fake_wstream *stream = (fake_wstream *)_stream;
 
 	if (stream->written + len > stream->size)
-		return GIT_ENOMEM;
+		return -1;
 
 	memcpy(stream->buffer + stream->written, data, len);
 	stream->written += len;
-	return GIT_SUCCESS;
+	return 0;
 }
 
 static void fake_wstream__free(git_odb_stream *_stream)
@@ -262,15 +254,14 @@ static int init_fake_wstream(git_odb_stream **stream_p, git_odb_backend *backend
 	fake_wstream *stream;
 
 	stream = git__calloc(1, sizeof(fake_wstream));
-	if (stream == NULL)
-		return GIT_ENOMEM;
+	GITERR_CHECK_ALLOC(stream);
 
 	stream->size = size;
 	stream->type = type;
 	stream->buffer = git__malloc(size);
 	if (stream->buffer == NULL) {
 		git__free(stream);
-		return GIT_ENOMEM;
+		return -1;
 	}
 
 	stream->stream.backend = backend;
@@ -281,7 +272,7 @@ static int init_fake_wstream(git_odb_stream **stream_p, git_odb_backend *backend
 	stream->stream.mode = GIT_STREAM_WRONLY;
 
 	*stream_p = (git_odb_stream *)stream;
-	return GIT_SUCCESS;
+	return 0;
 }
 
 /***********************************************************
@@ -305,26 +296,19 @@ static int backend_sort_cmp(const void *a, const void *b)
 
 int git_odb_new(git_odb **out)
 {
-	int error;
-
 	git_odb *db = git__calloc(1, sizeof(*db));
-	if (!db)
-		return GIT_ENOMEM;
+	GITERR_CHECK_ALLOC(db);
 
-	error = git_cache_init(&db->cache, GIT_DEFAULT_CACHE_SIZE, &free_odb_object);
-	if (error < GIT_SUCCESS) {
+	if (git_cache_init(&db->cache, GIT_DEFAULT_CACHE_SIZE, &free_odb_object) < 0 ||
+		git_vector_init(&db->backends, 4, backend_sort_cmp) < 0)
+	{
 		git__free(db);
-		return git__rethrow(error, "Failed to create object database");
-	}
-
-	if ((error = git_vector_init(&db->backends, 4, backend_sort_cmp)) < GIT_SUCCESS) {
-		git__free(db);
-		return git__rethrow(error, "Failed to create object database");
+		return -1;
 	}
 
 	*out = db;
 	GIT_REFCOUNT_INC(db);
-	return GIT_SUCCESS;
+	return 0;
 }
 
 static int add_backend_internal(git_odb *odb, git_odb_backend *backend, int priority, int is_alternate)
@@ -333,12 +317,11 @@ static int add_backend_internal(git_odb *odb, git_odb_backend *backend, int prio
 
 	assert(odb && backend);
 
-	if (backend->odb != NULL && backend->odb != odb)
-		return git__throw(GIT_EBUSY, "The backend is already owned by another ODB");
+	/* Check if the backend is already owned by another ODB */
+	assert(!backend->odb || backend->odb == odb);
 
 	internal = git__malloc(sizeof(backend_internal));
-	if (internal == NULL)
-		return GIT_ENOMEM;
+	GITERR_CHECK_ALLOC(internal);
 
 	internal->backend = backend;
 	internal->priority = priority;
@@ -346,12 +329,12 @@ static int add_backend_internal(git_odb *odb, git_odb_backend *backend, int prio
 
 	if (git_vector_insert(&odb->backends, internal) < 0) {
 		git__free(internal);
-		return GIT_ENOMEM;
+		return -1;
 	}
 
 	git_vector_sort(&odb->backends);
 	internal->backend->odb = odb;
-	return GIT_SUCCESS;
+	return 0;
 }
 
 int git_odb_add_backend(git_odb *odb, git_odb_backend *backend, int priority)
@@ -367,27 +350,18 @@ int git_odb_add_alternate(git_odb *odb, git_odb_backend *backend, int priority)
 static int add_default_backends(git_odb *db, const char *objects_dir, int as_alternates)
 {
 	git_odb_backend *loose, *packed;
-	int error;
 
 	/* add the loose object backend */
-	error = git_odb_backend_loose(&loose, objects_dir, -1, 0);
-	if (error < GIT_SUCCESS)
-		return error;
-
-	error = add_backend_internal(db, loose, GIT_LOOSE_PRIORITY, as_alternates);
-	if (error < GIT_SUCCESS)
-		return git__rethrow(error, "Failed to add backend");
+	if (git_odb_backend_loose(&loose, objects_dir, -1, 0) < 0 ||
+		add_backend_internal(db, loose, GIT_LOOSE_PRIORITY, as_alternates) < 0)
+		return -1;
 
 	/* add the packed file backend */
-	error = git_odb_backend_pack(&packed, objects_dir);
-	if (error < GIT_SUCCESS)
-		return error;
+	if (git_odb_backend_pack(&packed, objects_dir) < 0 ||
+		add_backend_internal(db, packed, GIT_PACKED_PRIORITY, as_alternates) < 0)
+		return -1;
 
-	error = add_backend_internal(db, packed, GIT_PACKED_PRIORITY, as_alternates);
-	if (error < GIT_SUCCESS)
-		return git__rethrow(error, "Failed to add backend");
-
-	return GIT_SUCCESS;
+	return 0;
 }
 
 static int load_alternates(git_odb *odb, const char *objects_dir)
@@ -396,24 +370,22 @@ static int load_alternates(git_odb *odb, const char *objects_dir)
 	git_buf alternates_buf = GIT_BUF_INIT;
 	char *buffer;
 	const char *alternate;
-	int error;
+	int result = 0;
 
-	error = git_buf_joinpath(&alternates_path, objects_dir, GIT_ALTERNATES_FILE);
-	if (error < GIT_SUCCESS)
-		return error;
+	if (git_buf_joinpath(&alternates_path, objects_dir, GIT_ALTERNATES_FILE) < 0)
+		return -1;
 
-	if (git_path_exists(alternates_path.ptr) < GIT_SUCCESS) {
+	if (git_path_exists(alternates_path.ptr) == false) {
 		git_buf_free(&alternates_path);
-		return GIT_SUCCESS;
+		return 0;
 	}
 
-	if (git_futils_readbuffer(&alternates_buf, alternates_path.ptr) < GIT_SUCCESS) {
+	if (git_futils_readbuffer(&alternates_buf, alternates_path.ptr) < 0) {
 		git_buf_free(&alternates_path);
-		return git__throw(GIT_EOSERR, "Failed to add backend. Can't read alternates");
+		return -1;
 	}
 
 	buffer = (char *)alternates_buf.ptr;
-	error = GIT_SUCCESS;
 
 	/* add each alternate as a new backend; one alternate per line */
 	while ((alternate = git__strtok(&buffer, "\r\n")) != NULL) {
@@ -422,48 +394,41 @@ static int load_alternates(git_odb *odb, const char *objects_dir)
 
 		/* relative path: build based on the current `objects` folder */
 		if (*alternate == '.') {
-			error = git_buf_joinpath(&alternates_path, objects_dir, alternate);
-			if (error < GIT_SUCCESS)
+			if ((result = git_buf_joinpath(&alternates_path, objects_dir, alternate)) < 0)
 				break;
 			alternate = git_buf_cstr(&alternates_path);
 		}
 
-		if ((error = add_default_backends(odb, alternate, 1)) < GIT_SUCCESS)
+		if ((result = add_default_backends(odb, alternate, 1)) < 0)
 			break;
 	}
 
 	git_buf_free(&alternates_path);
 	git_buf_free(&alternates_buf);
 
-	if (error < GIT_SUCCESS)
-		return git__rethrow(error, "Failed to load alternates");
-	return error;
+	return result;
 }
 
 int git_odb_open(git_odb **out, const char *objects_dir)
 {
 	git_odb *db;
-	int error;
 
 	assert(out && objects_dir);
 
 	*out = NULL;
 
-	if ((error = git_odb_new(&db)) < 0)
-		return git__rethrow(error, "Failed to open ODB");
+	if (git_odb_new(&db) < 0)
+		return -1;
 
-	if ((error = add_default_backends(db, objects_dir, 0)) < GIT_SUCCESS)
-		goto cleanup;
-
-	if ((error = load_alternates(db, objects_dir)) < GIT_SUCCESS)
-		goto cleanup;
+	if (add_default_backends(db, objects_dir, 0) < 0 ||
+		load_alternates(db, objects_dir) < 0)
+	{
+		git_odb_free(db);
+		return -1;
+	}
 
 	*out = db;
-	return GIT_SUCCESS;
-
-cleanup:
-	git_odb_free(db);
-	return error; /* error already set - pass through */
+	return 0;
 }
 
 static void odb_free(git_odb *db)
@@ -497,13 +462,13 @@ int git_odb_exists(git_odb *db, const git_oid *id)
 {
 	git_odb_object *object;
 	unsigned int i;
-	int found = 0;
+	bool found = false;
 
 	assert(db && id);
 
 	if ((object = git_cache_get(&db->cache, id)) != NULL) {
 		git_odb_object_free(object);
-		return 1;
+		return (int)true;
 	}
 
 	for (i = 0; i < db->backends.length && !found; ++i) {
@@ -514,7 +479,7 @@ int git_odb_exists(git_odb *db, const git_oid *id)
 			found = b->exists(b, id);
 	}
 
-	return found;
+	return (int)found;
 }
 
 int git_odb_read_header(size_t *len_p, git_otype *type_p, git_odb *db, const git_oid *id)
@@ -529,7 +494,7 @@ int git_odb_read_header(size_t *len_p, git_otype *type_p, git_odb *db, const git
 		*len_p = object->raw.len;
 		*type_p = object->raw.type;
 		git_odb_object_free(object);
-		return GIT_SUCCESS;
+		return 0;
 	}
 
 	for (i = 0; i < db->backends.length && error < 0; ++i) {
@@ -540,23 +505,20 @@ int git_odb_read_header(size_t *len_p, git_otype *type_p, git_odb *db, const git
 			error = b->read_header(len_p, type_p, b, id);
 	}
 
-	if (error == GIT_EPASSTHROUGH)
-		return GIT_SUCCESS;
+	if (!error || error == GIT_EPASSTHROUGH)
+		return 0;
 
 	/*
 	 * no backend could read only the header.
 	 * try reading the whole object and freeing the contents
 	 */
-	if (error < 0) {
-		if ((error = git_odb_read(&object, db, id)) < GIT_SUCCESS)
-			return error; /* error already set - pass through */
+	if ((error = git_odb_read(&object, db, id)) < 0)
+		return error; /* error already set - pass along */
 
-		*len_p = object->raw.len;
-		*type_p = object->raw.type;
-		git_odb_object_free(object);
-	}
-
-	return GIT_SUCCESS;
+	*len_p = object->raw.len;
+	*type_p = object->raw.type;
+	git_odb_object_free(object);
+	return 0;
 }
 
 int git_odb_read(git_odb_object **out, git_odb *db, const git_oid *id)
@@ -569,7 +531,7 @@ int git_odb_read(git_odb_object **out, git_odb *db, const git_oid *id)
 
 	*out = git_cache_get(&db->cache, id);
 	if (*out != NULL)
-		return GIT_SUCCESS;
+		return 0;
 
 	for (i = 0; i < db->backends.length && error < 0; ++i) {
 		backend_internal *internal = git_vector_get(&db->backends, i);
@@ -579,15 +541,19 @@ int git_odb_read(git_odb_object **out, git_odb *db, const git_oid *id)
 			error = b->read(&raw.data, &raw.len, &raw.type, b, id);
 	}
 
-	if (error == GIT_EPASSTHROUGH || error == GIT_SUCCESS) {
-		*out = git_cache_try_store(&db->cache, new_odb_object(id, &raw));
-		return GIT_SUCCESS;
-	}
+	/* TODO: If no backends are configured, this returns GIT_ENOTFOUND but
+	 * will never have called giterr_set().
+	 */
 
-	return git__rethrow(error, "Failed to read object");
+	if (error && error != GIT_EPASSTHROUGH)
+		return error;
+
+	*out = git_cache_try_store(&db->cache, new_odb_object(id, &raw));
+	return 0;
 }
 
-int git_odb_read_prefix(git_odb_object **out, git_odb *db, const git_oid *short_id, unsigned int len)
+int git_odb_read_prefix(
+	git_odb_object **out, git_odb *db, const git_oid *short_id, unsigned int len)
 {
 	unsigned int i;
 	int error = GIT_ENOTFOUND;
@@ -598,7 +564,7 @@ int git_odb_read_prefix(git_odb_object **out, git_odb *db, const git_oid *short_
 	assert(out && db);
 
 	if (len < GIT_OID_MINPREFIXLEN)
-		return git__throw(GIT_EAMBIGUOUSOIDPREFIX, "Failed to lookup object. Prefix length is lower than %d.", GIT_OID_MINPREFIXLEN);
+		return git_odb__error_ambiguous("prefix length too short");
 
 	if (len > GIT_OID_HEXSZ)
 		len = GIT_OID_HEXSZ;
@@ -606,7 +572,7 @@ int git_odb_read_prefix(git_odb_object **out, git_odb *db, const git_oid *short_
 	if (len == GIT_OID_HEXSZ) {
 		*out = git_cache_get(&db->cache, short_id);
 		if (*out != NULL)
-			return GIT_SUCCESS;
+			return 0;
 	}
 
 	for (i = 0; i < db->backends.length && found < 2; ++i) {
@@ -615,33 +581,24 @@ int git_odb_read_prefix(git_odb_object **out, git_odb *db, const git_oid *short_
 
 		if (b->read != NULL) {
 			error = b->read_prefix(&full_oid, &raw.data, &raw.len, &raw.type, b, short_id, len);
-			switch (error) {
-			case GIT_SUCCESS:
+			if (!error)
 				found++;
-				break;
-			case GIT_ENOTFOUND:
-			case GIT_EPASSTHROUGH:
-				break;
-			case GIT_EAMBIGUOUSOIDPREFIX:
-				return git__rethrow(error, "Failed to read object. Ambiguous sha1 prefix");
-			default:
-				return git__rethrow(error, "Failed to read object");
-			}
+			else if (error != GIT_ENOTFOUND && error != GIT_EPASSTHROUGH)
+				return error;
 		}
 	}
 
-	if (found == 1) {
-		*out = git_cache_try_store(&db->cache, new_odb_object(&full_oid, &raw));
-	} else if (found > 1) {
-		return git__throw(GIT_EAMBIGUOUSOIDPREFIX, "Failed to read object. Ambiguous sha1 prefix");
-	} else {
-		return git__throw(GIT_ENOTFOUND, "Failed to read object. Object not found");
-	}
+	if (found == 0)
+		return git_odb__error_notfound("no match for prefix", short_id);
+	if (found > 1)
+		return git_odb__error_ambiguous("multiple matches for prefix");
 
-	return GIT_SUCCESS;
+	*out = git_cache_try_store(&db->cache, new_odb_object(&full_oid, &raw));
+	return 0;
 }
 
-int git_odb_write(git_oid *oid, git_odb *db, const void *data, size_t len, git_otype type)
+int git_odb_write(
+	git_oid *oid, git_odb *db, const void *data, size_t len, git_otype type)
 {
 	unsigned int i;
 	int error = GIT_ERROR;
@@ -661,24 +618,25 @@ int git_odb_write(git_oid *oid, git_odb *db, const void *data, size_t len, git_o
 			error = b->write(oid, b, data, len, type);
 	}
 
-	if (error == GIT_EPASSTHROUGH || error == GIT_SUCCESS)
-		return GIT_SUCCESS;
+	if (!error || error == GIT_EPASSTHROUGH)
+		return 0;
 
 	/* if no backends were able to write the object directly, we try a streaming
 	 * write to the backends; just write the whole object into the stream in one
 	 * push */
 
-	if ((error = git_odb_open_wstream(&stream, db, len, type)) == GIT_SUCCESS) {
-		stream->write(stream, data, len);
-		error = stream->finalize_write(oid, stream);
-		stream->free(stream);
-		return GIT_SUCCESS;
-	}
+	if ((error = git_odb_open_wstream(&stream, db, len, type)) != 0)
+		return error;
 
-	return git__rethrow(error, "Failed to write object");
+	stream->write(stream, data, len);
+	error = stream->finalize_write(oid, stream);
+	stream->free(stream);
+
+	return error;
 }
 
-int git_odb_open_wstream(git_odb_stream **stream, git_odb *db, size_t size, git_otype type)
+int git_odb_open_wstream(
+	git_odb_stream **stream, git_odb *db, size_t size, git_otype type)
 {
 	unsigned int i;
 	int error = GIT_ERROR;
@@ -699,10 +657,10 @@ int git_odb_open_wstream(git_odb_stream **stream, git_odb *db, size_t size, git_
 			error = init_fake_wstream(stream, b, size, type);
 	}
 
-	if (error == GIT_EPASSTHROUGH || error == GIT_SUCCESS)
-		return GIT_SUCCESS;
+	if (error == GIT_EPASSTHROUGH)
+		error = 0;
 
-	return git__rethrow(error, "Failed to open write stream");
+	return error;
 }
 
 int git_odb_open_rstream(git_odb_stream **stream, git_odb *db, const git_oid *oid)
@@ -720,9 +678,27 @@ int git_odb_open_rstream(git_odb_stream **stream, git_odb *db, const git_oid *oi
 			error = b->readstream(stream, b, oid);
 	}
 
-	if (error == GIT_EPASSTHROUGH || error == GIT_SUCCESS)
-		return GIT_SUCCESS;
+	if (error == GIT_EPASSTHROUGH)
+		error = 0;
 
-	return git__rethrow(error, "Failed to open read stream");
+	return error;
+}
+
+int git_odb__error_notfound(const char *message, const git_oid *oid)
+{
+	if (oid != NULL) {
+		char oid_str[GIT_OID_HEXSZ + 1];
+		git_oid_tostr(oid_str, sizeof(oid_str), oid);
+		giterr_set(GITERR_ODB, "Object not found - %s (%s)", message, oid_str);
+	} else
+		giterr_set(GITERR_ODB, "Object not found - %s", message);
+
+	return GIT_ENOTFOUND;
+}
+
+int git_odb__error_ambiguous(const char *message)
+{
+	giterr_set(GITERR_ODB, "Ambiguous SHA1 prefix - %s", message);
+	return GIT_EAMBIGUOUS;
 }