Improve GIT_EUSER handling

This adds giterr_user_cancel to return GIT_EUSER and clear any
error message that is sitting around.  As a result of using that
in places, we need to be more thorough with capturing errors that
happen inside a callback when used internally.  To help with that,
this also adds giterr_capture and giterr_restore so that when we
internally use a foreach-type function that clears errors and
converts them to GIT_EUSER, it is easier to restore not just the
return value, but the actual error message text.

1 files changed, 68 insertions, 54 deletions

diff --git a/src/submodule.c b/src/submodule.c
index 1e3d07911..15c87c0b4 100644
--- a/src/submodule.c
+++ b/src/submodule.c
@@ -71,6 +71,11 @@ __KHASH_IMPL(
 	str, static kh_inline, const char *, void *, 1,
 	str_hash_no_trailing_slash, str_equal_no_trailing_slash);
 
+struct submodule_callback_payload {
+	git_repository *repo;
+	git_error_state error;
+};
+
 static int load_submodule_config(git_repository *repo);
 static git_config_backend *open_gitmodules(git_repository *, bool, const git_oid *);
 static int lookup_head_remote(git_buf *url, git_repository *repo);
@@ -169,8 +174,7 @@ int git_submodule_foreach(
 		}
 
 		if (callback(sm, sm->name, payload)) {
-			giterr_clear();
-			error = GIT_EUSER;
+			error = giterr_user_cancel();
 			break;
 		}
 	});
@@ -821,20 +825,21 @@ int git_submodule_reload(git_submodule *submodule)
 {
 	int error = 0;
 	git_config_backend *mods;
+	struct submodule_callback_payload p;
 
 	assert(submodule);
 
 	/* refresh index data */
-
-	if (submodule_update_index(submodule) < 0)
-		return -1;
+	if ((error = submodule_update_index(submodule)) < 0)
+		return error;
 
 	/* refresh HEAD tree data */
-
-	if (submodule_update_head(submodule) < 0)
-		return -1;
+	if ((error = submodule_update_head(submodule)) < 0)
+		return error;
 
 	/* refresh config data */
+	memset(&p, 0, sizeof(p));
+	p.repo = submodule->repo;
 
 	mods = open_gitmodules(submodule->repo, false, NULL);
 	if (mods != NULL) {
@@ -846,23 +851,29 @@ int git_submodule_reload(git_submodule *submodule)
 
 		if (git_buf_oom(&path))
 			error = -1;
-		else
+		else {
 			error = git_config_file_foreach_match(
-				mods, path.ptr, submodule_load_from_config, submodule->repo);
+				mods, path.ptr, submodule_load_from_config, &p);
+
+			if (error == GIT_EUSER)
+				error = giterr_restore(&p.error);
+		}
 
 		git_buf_free(&path);
 		git_config_file_free(mods);
-	}
 
-	if (error < 0)
-		return error;
+		if (error < 0)
+			return error;
+	}
 
 	/* refresh wd data */
 
 	submodule->flags = submodule->flags &
 		~(GIT_SUBMODULE_STATUS_IN_WD | GIT_SUBMODULE_STATUS__WD_OID_VALID);
 
-	error = submodule_load_from_wd_lite(submodule, submodule->path, NULL);
+	error = submodule_load_from_wd_lite(submodule, submodule->path, &p);
+	if (error)
+		error = giterr_restore(&p.error);
 
 	return error;
 }
@@ -1087,15 +1098,14 @@ int git_submodule_parse_update(git_submodule_update_t *out, const char *value)
 }
 
 static int submodule_load_from_config(
-	const git_config_entry *entry, void *data)
+	const git_config_entry *entry, void *payload)
 {
-	git_repository *repo = data;
-	git_strmap *smcfg = repo->submodules;
+	struct submodule_callback_payload *p = payload;
+	git_strmap *smcfg = p->repo->submodules;
 	const char *namestart, *property, *alternate = NULL;
-	const char *key = entry->name, *value = entry->value;
+	const char *key = entry->name, *value = entry->value, *path;
 	git_buf name = GIT_BUF_INIT;
 	git_submodule *sm;
-	bool is_path;
 	int error = 0;
 
 	if (git__prefixcmp(key, "submodule.") != 0)
@@ -1108,15 +1118,11 @@ static int submodule_load_from_config(
 		return 0;
 
 	property++;
-	is_path = (strcasecmp(property, "path") == 0);
+	path = !strcasecmp(property, "path") ? value : NULL;
 
-	if (git_buf_set(&name, namestart, property - namestart - 1) < 0)
-		return -1;
-
-	if (submodule_get(&sm, repo, name.ptr, is_path ? value : NULL) < 0) {
-		git_buf_free(&name);
-		return -1;
-	}
+	if ((error = git_buf_set(&name, namestart, property - namestart - 1)) < 0 ||
+		(error = submodule_get(&sm, p->repo, name.ptr, path)) < 0)
+		goto done;
 
 	sm->flags |= GIT_SUBMODULE_STATUS_IN_CONFIG;
 
@@ -1130,15 +1136,20 @@ static int submodule_load_from_config(
 
 	if (strcmp(sm->name, name.ptr) != 0) {
 		alternate = sm->name = git_buf_detach(&name);
-	} else if (is_path && value && strcmp(sm->path, value) != 0) {
+	} else if (path && strcmp(path, sm->path) != 0) {
 		alternate = sm->path = git__strdup(value);
-		if (!sm->path)
+		if (!sm->path) {
 			error = -1;
+			goto done;
+		}
 	}
+
 	if (alternate) {
 		void *old_sm = NULL;
 		git_strmap_insert2(smcfg, alternate, sm, old_sm, error);
 
+		if (error < 0)
+			goto done;
 		if (error >= 0)
 			GIT_REFCOUNT_INC(sm); /* inserted under a new key */
 
@@ -1149,15 +1160,11 @@ static int submodule_load_from_config(
 		}
 	}
 
-	git_buf_free(&name);
-	if (error < 0)
-		return error;
-
 	/* TODO: Look up path in index and if it is present but not a GITLINK
 	 * then this should be deleted (at least to match git's behavior)
 	 */
 
-	if (is_path)
+	if (path)
 		return 0;
 
 	/* copy other properties into submodule entry */
@@ -1165,41 +1172,47 @@ static int submodule_load_from_config(
 		git__free(sm->url);
 		sm->url = NULL;
 
-		if (value != NULL && (sm->url = git__strdup(value)) == NULL)
-			return -1;
+		if (value != NULL && (sm->url = git__strdup(value)) == NULL) {
+			error = -1;
+			goto done;
+		}
 	}
 	else if (strcasecmp(property, "update") == 0) {
-		if (git_submodule_parse_update(&sm->update, value) < 0)
-			return -1;
+		if ((error = git_submodule_parse_update(&sm->update, value)) < 0)
+			goto done;
 		sm->update_default = sm->update;
 	}
 	else if (strcasecmp(property, "fetchRecurseSubmodules") == 0) {
-		if (git__parse_bool(&sm->fetch_recurse, value) < 0)
-			return submodule_config_error("fetchRecurseSubmodules", value);
+		if (git__parse_bool(&sm->fetch_recurse, value) < 0) {
+			error = submodule_config_error("fetchRecurseSubmodules", value);
+			goto done;
+		}
 	}
 	else if (strcasecmp(property, "ignore") == 0) {
-		if (git_submodule_parse_ignore(&sm->ignore, value) < 0)
-			return -1;
+		if ((error = git_submodule_parse_ignore(&sm->ignore, value)) < 0)
+			goto done;
 		sm->ignore_default = sm->ignore;
 	}
 	/* ignore other unknown submodule properties */
 
-	return 0;
+done:
+	git_buf_free(&name);
+	return giterr_capture(&p->error, error);
 }
 
 static int submodule_load_from_wd_lite(
 	git_submodule *sm, const char *name, void *payload)
 {
+	struct submodule_callback_payload *p = payload;
 	git_buf path = GIT_BUF_INIT;
 
 	GIT_UNUSED(name);
-	GIT_UNUSED(payload);
 
 	if (git_repository_is_bare(sm->repo))
 		return 0;
 
 	if (git_buf_joinpath(&path, git_repository_workdir(sm->repo), sm->path) < 0)
-		return -1;
+		return giterr_capture(&p->error, -1);
 
 	if (git_path_isdir(path.ptr))
 		sm->flags |= GIT_SUBMODULE_STATUS__WD_SCANNED;
@@ -1208,7 +1221,6 @@ static int submodule_load_from_wd_lite(
 		sm->flags |= GIT_SUBMODULE_STATUS_IN_WD;
 
 	git_buf_free(&path);
-
 	return 0;
 }
 
@@ -1342,13 +1354,15 @@ static int load_submodule_config(git_repository *repo)
 {
 	int error;
 	git_oid gitmodules_oid;
-	git_buf path = GIT_BUF_INIT;
 	git_config_backend *mods = NULL;
+	struct submodule_callback_payload p;
 
 	if (repo->submodules)
 		return 0;
 
 	memset(&gitmodules_oid, 0, sizeof(gitmodules_oid));
+	memset(&p, 0, sizeof(p));
+	p.repo = repo;
 
 	/* Submodule data is kept in a hashtable keyed by both name and path.
 	 * These are usually the same, but that is not guaranteed.
@@ -1370,23 +1384,23 @@ static int load_submodule_config(git_repository *repo)
 
 	/* add submodule information from .gitmodules */
 
-	if ((mods = open_gitmodules(repo, false, &gitmodules_oid)) != NULL)
-		error = git_config_file_foreach(mods, submodule_load_from_config, repo);
-
-	if (error != 0)
+	if ((mods = open_gitmodules(repo, false, &gitmodules_oid)) != NULL &&
+		(error = git_config_file_foreach(
+			mods, submodule_load_from_config, &p)) < 0)
 		goto cleanup;
 
 	/* shallow scan submodules in work tree */
 
 	if (!git_repository_is_bare(repo))
-		error = git_submodule_foreach(repo, submodule_load_from_wd_lite, NULL);
+		error = git_submodule_foreach(repo, submodule_load_from_wd_lite, &p);
 
 cleanup:
-	git_buf_free(&path);
-
 	if (mods != NULL)
 		git_config_file_free(mods);
 
+	if (error == GIT_EUSER)
+		error = giterr_restore(&p.error);
+
 	if (error)
 		git_submodule_config_free(repo);