error-handling: remote, transport

3 files changed, 107 insertions, 150 deletions

diff --git a/include/git2/errors.h b/include/git2/errors.h
index 712d611ac..325d0a615 100644
--- a/include/git2/errors.h
+++ b/include/git2/errors.h
@@ -131,7 +131,8 @@ typedef enum {
 	GITERR_REGEX,
 	GITERR_ODB,
 	GITERR_INDEX,
-	GITERR_OBJECT
+	GITERR_OBJECT,
+	GITERR_NET,
 } git_error_class;
 
 /**
diff --git a/src/remote.c b/src/remote.c
index 52b6aacc9..b48a23339 100644
--- a/src/remote.c
+++ b/src/remote.c
@@ -29,30 +29,26 @@ static int refspec_parse(git_refspec *refspec, const char *str)
 	}
 
 	delim = strchr(str, ':');
-	if (delim == NULL)
-		return git__throw(GIT_EOBJCORRUPTED, "Failed to parse refspec. No ':'");
+	if (delim == NULL) {
+		giterr_set(GITERR_NET, "Invalid refspec, missing ':'");
+		return -1;
+	}
 
 	refspec->src = git__strndup(str, delim - str);
-	if (refspec->src == NULL)
-		return GIT_ENOMEM;
+	GITERR_CHECK_ALLOC(refspec->src);
 
 	refspec->dst = git__strdup(delim + 1);
-	if (refspec->dst == NULL) {
-		git__free(refspec->src);
-		refspec->src = NULL;
-		return GIT_ENOMEM;
-	}
+	GITERR_CHECK_ALLOC(refspec->dst);
 
-	return GIT_SUCCESS;
+	return 0;
 }
 
 static int parse_remote_refspec(git_config *cfg, git_refspec *refspec, const char *var)
 {
-	const char *val;
 	int error;
+	const char *val;
 
-	error = git_config_get_string(cfg, var, &val);
-	if (error < GIT_SUCCESS)
+	if ((error = git_config_get_string(cfg, var, &val)) < 0)
 		return error;
 
 	return refspec_parse(refspec, val);
@@ -66,33 +62,24 @@ int git_remote_new(git_remote **out, git_repository *repo, const char *url, cons
 	assert(out && repo && url);
 
 	remote = git__malloc(sizeof(git_remote));
-	if (remote == NULL)
-		return GIT_ENOMEM;
+	GITERR_CHECK_ALLOC(remote);
 
 	memset(remote, 0x0, sizeof(git_remote));
 	remote->repo = repo;
 
-	if (git_vector_init(&remote->refs, 32, NULL) < 0) {
-		git_remote_free(remote);
-		return GIT_ENOMEM;
-	}
+	if (git_vector_init(&remote->refs, 32, NULL) < 0)
+		return -1;
 
 	remote->url = git__strdup(url);
-	if (remote->url == NULL) {
-		git_remote_free(remote);
-		return GIT_ENOMEM;
-	}
+	GITERR_CHECK_ALLOC(remote->url);
 
 	if (name != NULL) {
 		remote->name = git__strdup(name);
-		if (remote->name == NULL) {
-			git_remote_free(remote);
-			return GIT_ENOMEM;
-		}
+		GITERR_CHECK_ALLOC(remote->name);
 	}
 
 	*out = remote;
-	return GIT_SUCCESS;
+	return 0;
 }
 
 int git_remote_load(git_remote **out, git_repository *repo, const char *name)
@@ -100,87 +87,68 @@ int git_remote_load(git_remote **out, git_repository *repo, const char *name)
 	git_remote *remote;
 	git_buf buf = GIT_BUF_INIT;
 	const char *val;
-	int error;
+	int error = 0;
 	git_config *config;
 
 	assert(out && repo && name);
 
-	error = git_repository_config__weakptr(&config, repo);
-	if (error < GIT_SUCCESS)
-		return error;
+	if (git_repository_config__weakptr(&config, repo) < 0)
+		return -1;
 
 	remote = git__malloc(sizeof(git_remote));
-	if (remote == NULL)
-		return GIT_ENOMEM;
+	GITERR_CHECK_ALLOC(remote);
 
 	memset(remote, 0x0, sizeof(git_remote));
 	remote->name = git__strdup(name);
-	if (remote->name == NULL) {
-		error = GIT_ENOMEM;
-		goto cleanup;
-	}
+	GITERR_CHECK_ALLOC(remote->name);
 
-	if (git_vector_init(&remote->refs, 32, NULL) < 0) {
-		error = GIT_ENOMEM;
-		goto cleanup;
-	}
+	if (git_vector_init(&remote->refs, 32, NULL) < 0)
+		return -1;
 
-	git_buf_printf(&buf, "remote.%s.url", name);
-	if (git_buf_oom(&buf)) {
-		error = GIT_ENOMEM;
-		goto cleanup;
-	}
+	if (git_buf_printf(&buf, "remote.%s.url", name) < 0)
+		return -1;
 
-	error = git_config_get_string(config, git_buf_cstr(&buf), &val);
-	if (error < GIT_SUCCESS) {
-		error = git__rethrow(error, "Remote's url doesn't exist");
+	if (git_config_get_string(config, git_buf_cstr(&buf), &val) < 0) {
+		error = -1;
 		goto cleanup;
 	}
 
 	remote->repo = repo;
 	remote->url = git__strdup(val);
-	if (remote->url == NULL) {
-		error = GIT_ENOMEM;
-		goto cleanup;
-	}
+	GITERR_CHECK_ALLOC(remote->url);
 
 	git_buf_clear(&buf);
-	git_buf_printf(&buf, "remote.%s.fetch", name);
-	if (git_buf_oom(&buf)) {
-		error = GIT_ENOMEM;
-		goto cleanup;
-	}
+	if (git_buf_printf(&buf, "remote.%s.fetch", name) < 0)
+		return -1;
 
 	error = parse_remote_refspec(config, &remote->fetch, git_buf_cstr(&buf));
 	if (error == GIT_ENOTFOUND)
-		error = GIT_SUCCESS;
+		error = 0;
 
-	if (error < GIT_SUCCESS) {
-		error = git__rethrow(error, "Failed to get fetch refspec");
+	if (error < 0) {
+		error = -1;
 		goto cleanup;
 	}
 
 	git_buf_clear(&buf);
-	git_buf_printf(&buf, "remote.%s.push", name);
-	if (git_buf_oom(&buf)) {
-		error = GIT_ENOMEM;
-		goto cleanup;
-	}
+	if (git_buf_printf(&buf, "remote.%s.push", name) < 0)
+		return -1;
 
 	error = parse_remote_refspec(config, &remote->push, git_buf_cstr(&buf));
-	/* Not finding push is fine */
 	if (error == GIT_ENOTFOUND)
-		error = GIT_SUCCESS;
+		error = 0;
 
-	if (error < GIT_SUCCESS)
+	if (error < 0) {
+		error = -1;
 		goto cleanup;
+	}
 
 	*out = remote;
 
 cleanup:
 	git_buf_free(&buf);
 
-	if (error < GIT_SUCCESS)
+	if (error < 0)
 		git_remote_free(remote);
 
 	return error;
@@ -188,52 +156,53 @@ cleanup:
 
 int git_remote_save(const git_remote *remote)
 {
-	int error;
 	git_config *config;
 	git_buf buf = GIT_BUF_INIT, value = GIT_BUF_INIT;
 
-	error = git_repository_config__weakptr(&config, remote->repo);
-	if (error < GIT_SUCCESS)
-		return error;
+	if (git_repository_config__weakptr(&config, remote->repo) < 0)
+		return -1;
 
-	git_buf_printf(&buf, "remote.%s.%s", remote->name, "url");
-	if (git_buf_oom(&buf))
-		return GIT_ENOMEM;
+	if (git_buf_printf(&buf, "remote.%s.%s", remote->name, "url") < 0)
+		return -1;
 
-	error = git_config_set_string(config, git_buf_cstr(&buf), remote->url);
-	if (error < GIT_SUCCESS)
-		goto cleanup;
+	if (git_config_set_string(config, git_buf_cstr(&buf), remote->url) < 0) {
+		git_buf_free(&buf);
+		return -1;
+	}
 
 	if (remote->fetch.src != NULL && remote->fetch.dst != NULL) {
 		git_buf_clear(&buf);
 		git_buf_clear(&value);
-		git_buf_printf(&buf, "remote.%s.%s", remote->name, "fetch");
+		git_buf_printf(&buf, "remote.%s.fetch", remote->name);
 		git_buf_printf(&value, "%s:%s", remote->fetch.src, remote->fetch.dst);
 		if (git_buf_oom(&buf) || git_buf_oom(&value))
-			return GIT_ENOMEM;
+			return -1;
 
-		error = git_config_set_string(config, git_buf_cstr(&buf), git_buf_cstr(&value));
-		if (error < GIT_SUCCESS)
-			goto cleanup;
+		if (git_config_set_string(config, git_buf_cstr(&buf), git_buf_cstr(&value)) < 0)
+			goto on_error;
 	}
 
 	if (remote->push.src != NULL && remote->push.dst != NULL) {
 		git_buf_clear(&buf);
 		git_buf_clear(&value);
-		git_buf_printf(&buf, "remote.%s.%s", remote->name, "push");
+		git_buf_printf(&buf, "remote.%s.push", remote->name);
 		git_buf_printf(&value, "%s:%s", remote->push.src, remote->push.dst);
 		if (git_buf_oom(&buf) || git_buf_oom(&value))
-			return GIT_ENOMEM;
+			return -1;
 
-		error = git_config_set_string(config, git_buf_cstr(&buf), git_buf_cstr(&value));
-		if (error < GIT_SUCCESS)
-			goto cleanup;
+		if (git_config_set_string(config, git_buf_cstr(&buf), git_buf_cstr(&value)) < 0)
+			goto on_error;
 	}
 
-cleanup:
 	git_buf_free(&buf);
 	git_buf_free(&value);
-	return error;
+
+	return 0;
+
+on_error:
+	git_buf_free(&buf);
+	git_buf_free(&value);
+	return -1;
 }
 
 const char *git_remote_name(git_remote *remote)
@@ -250,21 +219,19 @@ const char *git_remote_url(git_remote *remote)
 
 int git_remote_set_fetchspec(git_remote *remote, const char *spec)
 {
-	int error;
 	git_refspec refspec;
 
 	assert(remote && spec);
 
-	error = refspec_parse(&refspec, spec);
-	if (error != GIT_SUCCESS)
-		return error;
+	if (refspec_parse(&refspec, spec) < 0)
+		return -1;
 
 	git__free(remote->fetch.src);
 	git__free(remote->fetch.dst);
 	remote->fetch.src = refspec.src;
 	remote->fetch.dst = refspec.dst;
 
-	return GIT_SUCCESS;
+	return 0;
 }
 
 const git_refspec *git_remote_fetchspec(git_remote *remote)
@@ -275,21 +242,19 @@ const git_refspec *git_remote_fetchspec(git_remote *remote)
 
 int git_remote_set_pushspec(git_remote *remote, const char *spec)
 {
-	int error;
 	git_refspec refspec;
 
 	assert(remote && spec);
 
-	error = refspec_parse(&refspec, spec);
-	if (error != GIT_SUCCESS)
-		return error;
+	if (refspec_parse(&refspec, spec) < 0)
+		return -1;
 
 	git__free(remote->push.src);
 	git__free(remote->push.dst);
 	remote->push.src = refspec.src;
 	remote->push.dst = refspec.dst;
 
-	return GIT_SUCCESS;
+	return 0;
 }
 
 const git_refspec *git_remote_pushspec(git_remote *remote)
@@ -300,36 +265,34 @@ const git_refspec *git_remote_pushspec(git_remote *remote)
 
 int git_remote_connect(git_remote *remote, int direction)
 {
-	int error;
 	git_transport *t;
 
 	assert(remote);
 
-	error = git_transport_new(&t, remote->url);
-	if (error < GIT_SUCCESS)
-		return git__rethrow(error, "Failed to create transport");
+	if (git_transport_new(&t, remote->url) < 0)
+		return -1;
 
-	error = t->connect(t, direction);
-	if (error < GIT_SUCCESS) {
-		error = git__rethrow(error, "Failed to connect the transport");
-		goto cleanup;
+	if (t->connect(t, direction) < 0) {
+		goto on_error;
 	}
 
 	remote->transport = t;
 
-cleanup:
-	if (error < GIT_SUCCESS)
-		t->free(t);
+	return 0;
 
-	return error;
+on_error:
+	t->free(t);
+	return -1;
 }
 
 int git_remote_ls(git_remote *remote, git_headlist_cb list_cb, void *payload)
 {
 	assert(remote);
 
-	if (!remote->transport || !remote->transport->connected)
-		return git__throw(GIT_ERROR, "The remote is not connected");
+	if (!remote->transport || !remote->transport->connected) {
+		giterr_set(GITERR_NET, "The remote is not connected");
+		return -1;
+	}
 
 	return remote->transport->ls(remote->transport, list_cb, payload);
 }
@@ -341,14 +304,14 @@ int git_remote_download(char **filename, git_remote *remote)
 	assert(filename && remote);
 
 	if ((error = git_fetch_negotiate(remote)) < 0)
-		return git__rethrow(error, "Error negotiating");
+		return error;
 
 	return git_fetch_download_pack(filename, remote);
 }
 
 int git_remote_update_tips(git_remote *remote)
 {
-	int error = GIT_SUCCESS;
+	int error = 0;
 	unsigned int i = 0;
 	git_buf refname = GIT_BUF_INIT;
 	git_vector *refs = &remote->refs;
@@ -364,23 +327,20 @@ int git_remote_update_tips(git_remote *remote)
 	/* HEAD is only allowed to be the first in the list */
 	head = refs->contents[0];
 	if (!strcmp(head->name, GIT_HEAD_FILE)) {
-		error = git_reference_create_oid(&ref, remote->repo, GIT_FETCH_HEAD_FILE, &head->oid, 1);
-		i = 1;
-		if (error < GIT_SUCCESS)
-			return git__rethrow(error, "Failed to update FETCH_HEAD");
+		if (git_reference_create_oid(&ref, remote->repo, GIT_FETCH_HEAD_FILE, &head->oid, 1) < 0)
+			return -1;
 
+		i = 1;
 		git_reference_free(ref);
 	}
 
 	for (; i < refs->length; ++i) {
 		head = refs->contents[i];
 
-		error = git_refspec_transform_r(&refname, spec, head->name);
-		if (error < GIT_SUCCESS)
+		if (git_refspec_transform_r(&refname, spec, head->name) < 0)
 			break;
 
-		error = git_reference_create_oid(&ref, remote->repo, refname.ptr, &head->oid, 1);
-		if (error < GIT_SUCCESS)
+		if (git_reference_create_oid(&ref, remote->repo, refname.ptr, &head->oid, 1) < 0)
 			break;
 
 		git_reference_free(ref);
@@ -436,20 +396,17 @@ static int remote_list_cb(const char *name, const char *value, void *data_)
 	struct cb_data *data = (struct cb_data *)data_;
 	size_t nmatch = 2;
 	regmatch_t pmatch[2];
-	int error;
 	GIT_UNUSED(value);
 
 	if (!regexec(data->preg, name, nmatch, pmatch, 0)) {
 		char *remote_name = git__strndup(&name[pmatch[1].rm_so], pmatch[1].rm_eo - pmatch[1].rm_so);
-		if (remote_name == NULL)
-			return GIT_ENOMEM;
+		GITERR_CHECK_ALLOC(remote_name);
 
-		error = git_vector_insert(data->list, remote_name);
-		if (error < GIT_SUCCESS)
-			return error;
+		if (git_vector_insert(data->list, remote_name) < 0)
+			return -1;
 	}
 
-	return GIT_SUCCESS;
+	return 0;
 }
 
 int git_remote_list(git_strarray *remotes_list, git_repository *repo)
@@ -460,23 +417,22 @@ int git_remote_list(git_strarray *remotes_list, git_repository *repo)
 	struct cb_data data;
 	int error;
 
-	error = git_repository_config__weakptr(&cfg, repo);
-	if (error < GIT_SUCCESS)
-		return error;
+	if (git_repository_config__weakptr(&cfg, repo) < 0)
+		return -1;
 
-	error = git_vector_init(&list, 4, NULL);
-	if (error < GIT_SUCCESS)
-		return error;
+	if (git_vector_init(&list, 4, NULL) < 0)
+		return -1;
 
-	error = regcomp(&preg, "^remote\\.(.*)\\.url$", REG_EXTENDED);
-	if (error < 0)
-		return GIT_EOSERR;
+	if (regcomp(&preg, "^remote\\.(.*)\\.url$", REG_EXTENDED) < 0) {
+		giterr_set(GITERR_OS, "Remote catch regex failed to compile");
+		return -1;
+	}
 
 	data.list = &list;
 	data.preg = &preg;
 	error = git_config_foreach(cfg, remote_list_cb, &data);
 	regfree(&preg);
-	if (error < GIT_SUCCESS) {
+	if (error < 0) {
 		size_t i;
 		char *elem;
 		git_vector_foreach(&list, i, elem) {
@@ -490,5 +446,5 @@ int git_remote_list(git_strarray *remotes_list, git_repository *repo)
 	remotes_list->strings = (char **)list.contents;
 	remotes_list->count = list.length;
 
-	return GIT_SUCCESS;
+	return 0;
 }
diff --git a/src/transport.c b/src/transport.c
index 785ddc35d..8087b2e44 100644
--- a/src/transport.c
+++ b/src/transport.c
@@ -46,7 +46,8 @@ static git_transport_cb transport_find_fn(const char *url)
 int git_transport_dummy(git_transport **transport)
 {
 	GIT_UNUSED(transport);
-	return git__throw(GIT_ENOTIMPLEMENTED, "This protocol isn't implemented. Sorry");
+	giterr_set(GITERR_NET, "This transport isn't implemented. Sorry");
+	return -1;
 }
 
 int git_transport_new(git_transport **out, const char *url)
@@ -66,11 +67,10 @@ int git_transport_new(git_transport **out, const char *url)
 
 	error = fn(&transport);
 	if (error < GIT_SUCCESS)
-		return git__rethrow(error, "Failed to create new transport");
+		return error;
 
 	transport->url = git__strdup(url);
-	if (transport->url == NULL)
-		return GIT_ENOMEM;
+	GITERR_CHECK_ALLOC(transport->url);
 
 	*out = transport;