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.

include/git2/errors.h

@@ -8,7 +8,6 @@
 #define INCLUDE_git_errors_h__
 
 #include "common.h"
-#include "buffer.h"
 
 /**
  * @file git2/errors.h

src/attr.c

@@ -193,8 +193,7 @@ int git_attr_foreach(
 
 				error = callback(assign->name, assign->value, payload);
 				if (error) {
-					giterr_clear();
-					error = GIT_EUSER;
+					error = giterr_user_cancel();
 					goto cleanup;
 				}
 			}

src/blame.c

@@ -108,17 +108,23 @@ git_blame* git_blame__alloc(
 	git_blame_options opts,
 	const char *path)
 {
-	git_blame *gbr = (git_blame*)git__calloc(1, sizeof(git_blame));
-	if (!gbr) {
-		giterr_set_oom();
+	git_blame *gbr = git__calloc(1, sizeof(git_blame));
+	if (!gbr)
 		return NULL;
-	}
-	git_vector_init(&gbr->hunks, 8, hunk_cmp);
-	git_vector_init(&gbr->paths, 8, paths_cmp);
+
 	gbr->repository = repo;
 	gbr->options = opts;
-	gbr->path = git__strdup(path);
-	git_vector_insert(&gbr->paths, git__strdup(path));
+
+	if (git_vector_init(&gbr->hunks, 8, hunk_cmp) < 0 ||
+		git_vector_init(&gbr->paths, 8, paths_cmp) < 0 ||
+		(gbr->path = git__strdup(path)) == NULL ||
+		git_vector_insert(&gbr->paths, git__strdup(path)) < 0)
+	{
+		git_blame_free(gbr);
+		git__free(gbr);
+		return NULL;
+	}
+
 	return gbr;
 }
 
@@ -140,7 +146,7 @@ void git_blame_free(git_blame *blame)
 
 	git_array_clear(blame->line_index);
 
-	git__free((void*)blame->path);
+	git__free(blame->path);
 	git_blob_free(blame->final_blob);
 	git__free(blame);
 }

src/blame.h

@@ -64,7 +64,7 @@ typedef struct git_blame__entry {
 } git_blame__entry;
 
 struct git_blame {
-	const char *path;
+	char *path;
 	git_repository *repository;
 	git_blame_options options;
 

src/branch.c

@@ -90,28 +90,27 @@ int git_branch_delete(git_reference *branch)
 
 	assert(branch);
 
-	if (!git_reference_is_branch(branch) &&
-		!git_reference_is_remote(branch)) {
-		giterr_set(GITERR_INVALID, "Reference '%s' is not a valid branch.", git_reference_name(branch));
-		return -1;
+	if (!git_reference_is_branch(branch) && !git_reference_is_remote(branch)) {
+		giterr_set(GITERR_INVALID, "Reference '%s' is not a valid branch.",
+			git_reference_name(branch));
+		return GIT_ENOTFOUND;
 	}
 
 	if ((is_head = git_branch_is_head(branch)) < 0)
 		return is_head;
 
 	if (is_head) {
-		giterr_set(GITERR_REFERENCE,
-				"Cannot delete branch '%s' as it is the current HEAD of the repository.", git_reference_name(branch));
+		giterr_set(GITERR_REFERENCE, "Cannot delete branch '%s' as it is "
+			"the current HEAD of the repository.", git_reference_name(branch));
 		return -1;
 	}
 
-	if (git_buf_printf(&config_section, "branch.%s", git_reference_name(branch) + strlen(GIT_REFS_HEADS_DIR)) < 0)
+	if (git_buf_join(&config_section, '.', "branch",
+			git_reference_name(branch) + strlen(GIT_REFS_HEADS_DIR)) < 0)
 		goto on_error;
 
 	if (git_config_rename_section(
-		git_reference_owner(branch), 
-		git_buf_cstr(&config_section),
-		NULL) < 0)
+		git_reference_owner(branch), git_buf_cstr(&config_section), NULL) < 0)
 		goto on_error;
 
 	if (git_reference_delete(branch) < 0)
@@ -206,17 +205,21 @@ int git_branch_move(
 	if (error < 0)
 		goto done;
 
-	git_buf_printf(&old_config_section,
-		"branch.%s", git_reference_name(branch) + strlen(GIT_REFS_HEADS_DIR));
-
-	git_buf_printf(&new_config_section, "branch.%s", new_branch_name);
+	/* first update ref then config so failure won't trash config */
 
-	if ((error = git_config_rename_section(git_reference_owner(branch),
-		git_buf_cstr(&old_config_section),
-		git_buf_cstr(&new_config_section))) < 0)
+	error = git_reference_rename(
+		out, branch, git_buf_cstr(&new_reference_name), force);
+	if (error < 0)
 		goto done;
 
-	error = git_reference_rename(out, branch, git_buf_cstr(&new_reference_name), force);
+	git_buf_join(&old_config_section, '.', "branch",
+		git_reference_name(branch) + strlen(GIT_REFS_HEADS_DIR));
+	git_buf_join(&new_config_section, '.', "branch", new_branch_name);
+
+	error = git_config_rename_section(
+		git_reference_owner(branch),
+		git_buf_cstr(&old_config_section),
+		git_buf_cstr(&new_config_section));
 
 done:
 	git_buf_free(&new_reference_name);

src/checkout.c

@@ -216,7 +216,7 @@ static int checkout_action_common(
 
 	if (notify != GIT_CHECKOUT_NOTIFY_NONE &&
 		checkout_notify(data, notify, delta, wd) != 0)
-		return GIT_EUSER;
+		return giterr_user_cancel();
 
 	return action;
 }
@@ -230,7 +230,7 @@ static int checkout_action_no_wd(
 	switch (delta->status) {
 	case GIT_DELTA_UNMODIFIED: /* case 12 */
 		if (checkout_notify(data, GIT_CHECKOUT_NOTIFY_DIRTY, delta, NULL))
-			return GIT_EUSER;
+			return giterr_user_cancel();
 		action = CHECKOUT_ACTION_IF(SAFE_CREATE, UPDATE_BLOB, NONE);
 		break;
 	case GIT_DELTA_ADDED:    /* case 2 or 28 (and 5 but not really) */
@@ -302,7 +302,7 @@ static int checkout_action_wd_only(
 	}
 
 	if (checkout_notify(data, notify, NULL, wd))
-		return GIT_EUSER;
+		return giterr_user_cancel();
 
 	if (remove) {
 		char *path = git_pool_strdup(&data->pool, wd->path);
@@ -342,7 +342,7 @@ static int checkout_action_with_wd(
 		if (checkout_is_workdir_modified(data, &delta->old_file, wd)) {
 			if (checkout_notify(
 					data, GIT_CHECKOUT_NOTIFY_DIRTY, delta, wd))
-				return GIT_EUSER;
+				return giterr_user_cancel();
 			action = CHECKOUT_ACTION_IF(FORCE, UPDATE_BLOB, NONE);
 		}
 		break;
@@ -406,7 +406,7 @@ static int checkout_action_with_wd_blocker(
 	case GIT_DELTA_UNMODIFIED:
 		/* should show delta as dirty / deleted */
 		if (checkout_notify(data, GIT_CHECKOUT_NOTIFY_DIRTY, delta, wd))
-			return GIT_EUSER;
+			return giterr_user_cancel();
 		action = CHECKOUT_ACTION_IF(FORCE, REMOVE_AND_UPDATE, NONE);
 		break;
 	case GIT_DELTA_ADDED:
@@ -439,7 +439,7 @@ static int checkout_action_with_wd_dir(
 		if (checkout_notify(data, GIT_CHECKOUT_NOTIFY_DIRTY, delta, NULL) ||
 			checkout_notify(
 				data, GIT_CHECKOUT_NOTIFY_UNTRACKED, NULL, wd))
-			return GIT_EUSER;
+			return giterr_user_cancel();
 		break;
 	case GIT_DELTA_ADDED:/* case 4 (and 7 for dir) */
 	case GIT_DELTA_MODIFIED: /* case 20 (or 37 but not really) */
@@ -452,7 +452,7 @@ static int checkout_action_with_wd_dir(
 		if (delta->old_file.mode != GIT_FILEMODE_TREE &&
 			checkout_notify(
 				data, GIT_CHECKOUT_NOTIFY_UNTRACKED, NULL, wd))
-			return GIT_EUSER;
+			return giterr_user_cancel();
 		break;
 	case GIT_DELTA_TYPECHANGE: /* case 24 or 31 */
 		if (delta->old_file.mode == GIT_FILEMODE_TREE) {
@@ -1998,9 +1998,6 @@ int git_checkout_iterator(
 	assert(data.completed_steps == data.total_steps);
 
 cleanup:
-	if (error == GIT_EUSER)
-		giterr_clear();
-
 	if (!error && data.index != NULL &&
 		(data.strategy & GIT_CHECKOUT_DONT_UPDATE_INDEX) == 0)
 		error = git_index_write(data.index);

src/common.h

@@ -78,28 +78,42 @@ int giterr_set_regex(const regex_t *regex, int error_code);
 /**
  * Gets the system error code for this thread.
  */
-GIT_INLINE(int) giterr_system_last(void)
-{
-#ifdef GIT_WIN32
-	return GetLastError();
-#else
-	return errno;
-#endif
-}
+int giterr_system_last(void);
 
 /**
  * Sets the system error code for this thread.
  */
-GIT_INLINE(void) giterr_system_set(int code)
+void giterr_system_set(int code);
+
+/**
+ * Note that a user cancelled an operation with GIT_EUSER
+ */
+GIT_INLINE(int) giterr_user_cancel(void)
 {
-#ifdef GIT_WIN32
-	SetLastError(code);
-#else
-	errno = code;
-#endif
+	giterr_clear();
+	return GIT_EUSER;
 }
 
 /**
+ * Structure to preserve libgit2 error state
+ */
+typedef struct {
+	int       error_code;
+	git_error error_msg;
+} git_error_state;
+
+/**
+ * Capture current error state to restore later, returning error code.
+ * If `error_code` is zero, this does nothing and returns zero.
+ */
+int giterr_capture(git_error_state *state, int error_code);
+
+/**
+ * Restore error state to a previous value, returning saved error code.
+ */
+int giterr_restore(git_error_state *state);
+
+/**
  * Check a versioned structure for validity
  */
 GIT_INLINE(int) giterr__check_version(const void *structure, unsigned int expected_max, const char *name)

src/config.c

@@ -501,20 +501,18 @@ int git_config_backend_foreach_match(
 		return -1;
 	}
 
-	while(!(iter->next(&entry, iter) < 0)) {
+	while (!(iter->next(&entry, iter) < 0)) {
 		/* skip non-matching keys if regexp was provided */
 		if (regexp && regexec(&regex, entry->name, 0, NULL, 0) != 0)
 			continue;
 
 		/* abort iterator on non-zero return value */
 		if (fn(entry, data)) {
-			giterr_clear();
-			result = GIT_EUSER;
-			goto cleanup;
+			result = giterr_user_cancel();
+			break;
 		}
 	}
 
-cleanup:
 	if (regexp != NULL)
 		regfree(&regex);
 
@@ -537,9 +535,8 @@ int git_config_foreach_match(
 		return error;
 
 	while ((error = git_config_next(&entry, iter)) == 0) {
-		if(cb(entry, payload)) {
-			giterr_clear();
-			error = GIT_EUSER;
+		if (cb(entry, payload)) {
+			error = giterr_user_cancel();
 			break;
 		}
 	}
@@ -800,9 +797,10 @@ int git_config_get_multivar_foreach(
 	found = 0;
 	while ((err = iter->next(&entry, iter)) == 0) {
 		found = 1;
-		if(cb(entry, payload)) {
+
+		if (cb(entry, payload)) {
 			iter->free(iter);
-			return GIT_EUSER;
+			return giterr_user_cancel();
 		}
 	}
 
@@ -1214,7 +1212,7 @@ struct rename_data {
 	git_config *config;
 	git_buf *name;
 	size_t old_len;
-	int actual_error;
+	git_error_state error;
 };
 
 static int rename_config_entries_cb(
@@ -1237,9 +1235,8 @@ static int rename_config_entries_cb(
 	if (!error)
 		error = git_config_delete_entry(data->config, entry->name);
 
-	data->actual_error = error; /* preserve actual error code */
-
-	return error;
+	/* capture error message as needed, since it will become EUSER */
+	return giterr_capture(&data->error, error);
 }
 
 int git_config_rename_section(
@@ -1260,10 +1257,10 @@ int git_config_rename_section(
 	if ((error = git_repository_config__weakptr(&config, repo)) < 0)
 		goto cleanup;
 
+	memset(&data, 0, sizeof(data));
 	data.config  = config;
 	data.name    = &replace;
 	data.old_len = strlen(old_section_name) + 1;
-	data.actual_error = 0;
 
 	if ((error = git_buf_join(&replace, '.', new_section_name, "")) < 0)
 		goto cleanup;
@@ -1281,7 +1278,7 @@ int git_config_rename_section(
 		config, git_buf_cstr(&pattern), rename_config_entries_cb, &data);
 
 	if (error == GIT_EUSER)
-		error = data.actual_error;
+		error = giterr_restore(&data.error);
 
 cleanup:
 	git_buf_free(&pattern);

src/diff.c

@@ -120,7 +120,7 @@ static int diff_delta__from_one(
 		return -1;
 	}
 
-	return notify_res < 0 ? GIT_EUSER : 0;
+	return notify_res < 0 ? giterr_user_cancel() : 0;
 }
 
 static int diff_delta__from_two(
@@ -182,7 +182,7 @@ static int diff_delta__from_two(
 		return -1;
 	}
 
-	return notify_res < 0 ? GIT_EUSER : 0;
+	return notify_res < 0 ? giterr_user_cancel() : 0;
 }
 
 static git_diff_delta *diff_delta__last_for_item(
@@ -1343,7 +1343,7 @@ int git_diff__paired_foreach(
 	int (*cb)(git_diff_delta *h2i, git_diff_delta *i2w, void *payload),
 	void *payload)
 {
-	int cmp;
+	int cmp, error = 0;
 	git_diff_delta *h2i, *i2w;
 	size_t i, j, i_max, j_max;
 	int (*strcomp)(const char *, const char *) = git__strcmp;
@@ -1399,18 +1399,17 @@ int git_diff__paired_foreach(
 			strcomp(h2i->new_file.path, i2w->old_file.path);
 
 		if (cmp < 0) {
-			if (cb(h2i, NULL, payload))
-				return GIT_EUSER;
-			i++;
+			i++; i2w = NULL;
 		} else if (cmp > 0) {
-			if (cb(NULL, i2w, payload))
-				return GIT_EUSER;
-			j++;
+			j++; h2i = NULL;
 		} else {
-			if (cb(h2i, i2w, payload))
-				return GIT_EUSER;
 			i++; j++;
 		}
+
+		if (cb(h2i, i2w, payload)) {
+			error = giterr_user_cancel();
+			break;
+		}
 	}
 
 	/* restore case-insensitive delta sort */
@@ -1426,5 +1425,5 @@ int git_diff__paired_foreach(
 		git_vector_sort(&idx2wd->deltas);
 	}
 
-	return 0;
+	return error;
 }

src/diff_patch.c

@@ -32,6 +32,7 @@ struct git_patch {
 	git_array_t(git_diff_line)   lines;
 	size_t content_size, context_size, header_size;
 	git_pool flattened;
+	git_error_state error;
 };
 
 enum {
@@ -193,21 +194,17 @@ cleanup:
 	return error;
 }
 
-static int diff_patch_file_callback(
+static int diff_patch_invoke_file_callback(
 	git_patch *patch, git_diff_output *output)
 {
-	float progress;
-
-	if (!output->file_cb)
-		return 0;
-
-	progress = patch->diff ?
+	float progress = patch->diff ?
 		((float)patch->delta_index / patch->diff->deltas.length) : 1.0f;
 
-	if (output->file_cb(patch->delta, progress, output->payload) != 0)
-		output->error = GIT_EUSER;
+	if (output->file_cb &&
+		output->file_cb(patch->delta, progress, output->payload) != 0)
+		return giterr_user_cancel();
 
-	return output->error;
+	return 0;
 }
 
 static int diff_patch_generate(git_patch *patch, git_diff_output *output)
@@ -229,7 +226,7 @@ static int diff_patch_generate(git_patch *patch, git_diff_output *output)
 		return 0;
 
 	if (output->diff_cb != NULL &&
-		!(error = output->diff_cb(output, patch)))
+		(error = output->diff_cb(output, patch)) < 0)
 		patch->flags |= GIT_DIFF_PATCH_DIFFED;
 
 	return error;
@@ -272,9 +269,10 @@ int git_diff_foreach(
 	size_t idx;
 	git_patch patch;
 
-	if (diff_required(diff, "git_diff_foreach") < 0)
-		return -1;
+	if ((error = diff_required(diff, "git_diff_foreach")) < 0)
+		return error;
 
+	memset(&xo, 0, sizeof(xo));
 	diff_output_init(
 		&xo.output, &diff->opts, file_cb, hunk_cb, data_cb, payload);
 	git_xdiff_init(&xo, &diff->opts);
@@ -285,22 +283,18 @@ int git_diff_foreach(
 		if (git_diff_delta__should_skip(&diff->opts, patch.delta))
 			continue;
 
-		if (!(error = diff_patch_init_from_diff(&patch, diff, idx))) {
-
-			error = diff_patch_file_callback(&patch, &xo.output);
+		if ((error = diff_patch_init_from_diff(&patch, diff, idx)) < 0)
+			break;
 
-			if (!error)
+		if (!(error = diff_patch_invoke_file_callback(&patch, &xo.output)))
 			error = diff_patch_generate(&patch, &xo.output);
 
 		git_patch_free(&patch);
-		}
 
 		if (error < 0)
 			break;
 	}
 
-	if (error == GIT_EUSER)
-		giterr_clear(); /* don't leave error message set invalidly */
 	return error;
 }
 
@@ -332,7 +326,7 @@ static int diff_single_generate(diff_patch_with_delta *pd, git_xdiff_output *xo)
 		!(patch->ofile.opts_flags & GIT_DIFF_INCLUDE_UNMODIFIED))
 		return error;
 
-	error = diff_patch_file_callback(patch, (git_diff_output *)xo);
+	error = diff_patch_invoke_file_callback(patch, (git_diff_output *)xo);
 
 	if (!error)
 		error = diff_patch_generate(patch, (git_diff_output *)xo);
@@ -424,9 +418,7 @@ int git_diff_blobs(
 	diff_patch_with_delta pd;
 	git_xdiff_output xo;
 
-	memset(&pd, 0, sizeof(pd));
 	memset(&xo, 0, sizeof(xo));
-
 	diff_output_init(
 		&xo.output, opts, file_cb, hunk_cb, data_cb, payload);
 	git_xdiff_init(&xo, opts);
@@ -436,6 +428,7 @@ int git_diff_blobs(
 	else if (!new_path && old_path)
 		new_path = old_path;
 
+	memset(&pd, 0, sizeof(pd));
 	error = diff_patch_from_blobs(
 		&pd, &xo, old_blob, old_path, new_blob, new_path, opts);
 
@@ -463,13 +456,15 @@ int git_patch_from_blobs(
 		return -1;
 
 	memset(&xo, 0, sizeof(xo));
-
 	diff_output_to_patch(&xo.output, &pd->patch);
 	git_xdiff_init(&xo, opts);
 
 	error = diff_patch_from_blobs(
 		pd, &xo, old_blob, old_path, new_blob, new_path, opts);
 
+	if (error == GIT_EUSER)
+		error = giterr_restore(&pd->patch.error);
+
 	if (!error)
 		*out = (git_patch *)pd;
 	else
@@ -536,9 +531,7 @@ int git_diff_blob_to_buffer(
 	diff_patch_with_delta pd;
 	git_xdiff_output xo;
 
-	memset(&pd, 0, sizeof(pd));
 	memset(&xo, 0, sizeof(xo));
-
 	diff_output_init(
 		&xo.output, opts, file_cb, hunk_cb, data_cb, payload);
 	git_xdiff_init(&xo, opts);
@@ -548,6 +541,7 @@ int git_diff_blob_to_buffer(
 	else if (!buf_path && old_path)
 		buf_path = old_path;
 
+	memset(&pd, 0, sizeof(pd));
 	error = diff_patch_from_blob_and_buffer(
 		&pd, &xo, old_blob, old_path, buf, buflen, buf_path, opts);
 
@@ -576,13 +570,15 @@ int git_patch_from_blob_and_buffer(
 		return -1;
 
 	memset(&xo, 0, sizeof(xo));
-
 	diff_output_to_patch(&xo.output, &pd->patch);
 	git_xdiff_init(&xo, opts);
 
 	error = diff_patch_from_blob_and_buffer(
 		pd, &xo, old_blob, old_path, buf, buflen, buf_path, opts);
 
+	if (error == GIT_EUSER)
+		error = giterr_restore(&pd->patch.error);
+
 	if (!error)
 		*out = (git_patch *)pd;
 	else
@@ -622,14 +618,18 @@ int git_patch_from_diff(
 	if ((error = diff_patch_alloc_from_diff(&patch, diff, idx)) < 0)
 		return error;
 
+	memset(&xo, 0, sizeof(xo));
 	diff_output_to_patch(&xo.output, patch);
 	git_xdiff_init(&xo, &diff->opts);
 
-	error = diff_patch_file_callback(patch, &xo.output);
+	error = diff_patch_invoke_file_callback(patch, &xo.output);
 
 	if (!error)
 		error = diff_patch_generate(patch, &xo.output);
 
+	if (error == GIT_EUSER)
+		error = giterr_restore(&patch->error);
+
 	if (!error) {
 		/* if cumulative diff size is < 0.5 total size, flatten the patch */
 		/* unload the file content */
@@ -879,7 +879,8 @@ static int diff_patch_hunk_cb(
 	GIT_UNUSED(delta);
 
 	hunk = git_array_alloc(patch->hunks);
-	GITERR_CHECK_ALLOC(hunk);
+	if (!hunk)
+		return giterr_capture(&patch->error, -1);
 
 	memcpy(&hunk->hunk, hunk_, sizeof(hunk->hunk));
 
@@ -905,10 +906,11 @@ static int diff_patch_line_cb(
 	GIT_UNUSED(hunk_);
 
 	hunk = git_array_last(patch->hunks);
-	GITERR_CHECK_ALLOC(hunk);
+	assert(hunk); /* programmer error if no hunk is available */
 
 	line = git_array_alloc(patch->lines);
-	GITERR_CHECK_ALLOC(line);
+	if (!line)
+		return giterr_capture(&patch->error, -1);
 
 	memcpy(line, line_, sizeof(*line));
 

src/diff_print.c

@@ -18,6 +18,7 @@ typedef struct {
 	uint32_t flags;
 	int oid_strlen;
 	git_diff_line line;
+	git_error_state error;
 } diff_print_info;
 
 static int diff_print_info_init(
@@ -33,6 +34,7 @@ static int diff_print_info_init(
 	pi->print_cb = cb;
 	pi->payload  = payload;
 	pi->buf      = out;
+	memset(&pi->error, 0, sizeof(pi->error));
 
 	if (diff)
 		pi->flags = diff->opts.flags;
@@ -89,12 +91,6 @@ char git_diff_status_char(git_delta_t status)
 	return code;
 }
 
-static int callback_error(void)
-{
-	giterr_clear();
-	return GIT_EUSER;
-}
-
 static int diff_print_one_name_only(
 	const git_diff_delta *delta, float progress, void *data)
 {
@@ -111,14 +107,14 @@ static int diff_print_one_name_only(
 
 	if (git_buf_puts(out, delta->new_file.path) < 0 ||
 		git_buf_putc(out, '\n'))
-		return -1;
+		return giterr_capture(&pi->error, -1);
 
 	pi->line.origin      = GIT_DIFF_LINE_FILE_HDR;
 	pi->line.content     = git_buf_cstr(out);
 	pi->line.content_len = git_buf_len(out);
 
 	if (pi->print_cb(delta, NULL, &pi->line, pi->payload))
-		return callback_error();
+		return giterr_user_cancel();
 
 	return 0;
 }
@@ -156,14 +152,14 @@ static int diff_print_one_name_status(
 		git_buf_printf(out, "%c\t%s\n", code, delta->old_file.path);
 
 	if (git_buf_oom(out))
-		return -1;
+		return giterr_capture(&pi->error, -1);
 
 	pi->line.origin      = GIT_DIFF_LINE_FILE_HDR;
 	pi->line.content     = git_buf_cstr(out);
 	pi->line.content_len = git_buf_len(out);
 
 	if (pi->print_cb(delta, NULL, &pi->line, pi->payload))
-		return callback_error();
+		return giterr_user_cancel();
 
 	return 0;
 }
@@ -202,14 +198,14 @@ static int diff_print_one_raw(
 			delta->old_file.path : delta->new_file.path);
 
 	if (git_buf_oom(out))
-		return -1;
+		return giterr_capture(&pi->error, -1);
 
 	pi->line.origin      = GIT_DIFF_LINE_FILE_HDR;
 	pi->line.content     = git_buf_cstr(out);
 	pi->line.content_len = git_buf_len(out);
 
 	if (pi->print_cb(delta, NULL, &pi->line, pi->payload))
-		return callback_error();
+		return giterr_user_cancel();
 
 	return 0;
 }
@@ -315,14 +311,14 @@ static int diff_print_patch_file(
 
 	if (git_diff_delta__format_file_header(
 			pi->buf, delta, oldpfx, newpfx, pi->oid_strlen) < 0)
-		return -1;
+		return giterr_capture(&pi->error, -1);
 
 	pi->line.origin      = GIT_DIFF_LINE_FILE_HDR;
 	pi->line.content     = git_buf_cstr(pi->buf);
 	pi->line.content_len = git_buf_len(pi->buf);
 
 	if (pi->print_cb(delta, NULL, &pi->line, pi->payload))
-		return callback_error();
+		return giterr_user_cancel();
 
 	if ((delta->flags & GIT_DIFF_FLAG_BINARY) == 0)
 		return 0;
@@ -332,7 +328,7 @@ static int diff_print_patch_file(
 	if (diff_delta_format_with_paths(
 			pi->buf, delta, oldpfx, newpfx,
 			"Binary files %s%s and %s%s differ\n") < 0)
-		return -1;
+		return giterr_capture(&pi->error, -1);
 
 	pi->line.origin      = GIT_DIFF_LINE_BINARY;
 	pi->line.content     = git_buf_cstr(pi->buf);
@@ -340,7 +336,7 @@ static int diff_print_patch_file(
 	pi->line.num_lines   = 1;
 
 	if (pi->print_cb(delta, NULL, &pi->line, pi->payload))
-		return callback_error();
+		return giterr_user_cancel();
 
 	return 0;
 }
@@ -360,7 +356,7 @@ static int diff_print_patch_hunk(
 	pi->line.content_len = h->header_len;
 
 	if (pi->print_cb(d, h, &pi->line, pi->payload))
-		return callback_error();
+		return giterr_user_cancel();
 
 	return 0;
 }
@@ -377,7 +373,7 @@ static int diff_print_patch_line(
 		return 0;
 
 	if (pi->print_cb(delta, hunk, line, pi->payload))
-		return callback_error();
+		return giterr_user_cancel();
 
 	return 0;
 }
@@ -421,9 +417,14 @@ int git_diff_print(
 
 	if (!(error = diff_print_info_init(
 			&pi, &buf, diff, format, print_cb, payload)))
+	{
 		error = git_diff_foreach(
 			diff, print_file, print_hunk, print_line, &pi);
 
+		if (error == GIT_EUSER && pi.error.error_code)
+			error = giterr_restore(&pi.error);
+	}
+
 	git_buf_free(&buf);
 
 	return error;
@@ -444,10 +445,15 @@ int git_patch_print(
 	if (!(error = diff_print_info_init(
 			&pi, &temp, git_patch__diff(patch),
 			GIT_DIFF_FORMAT_PATCH, print_cb, payload)))
+	{
 		error = git_patch__invoke_callbacks(
 			patch, diff_print_patch_file, diff_print_patch_hunk,
 			diff_print_patch_line, &pi);
 
+		if (error && error != GIT_EUSER)
+			error = giterr_restore(&pi.error);
+	}
+
 	git_buf_free(&temp);
 
 	return error;
@@ -483,8 +489,10 @@ int git_patch_to_str(
 	/* GIT_EUSER means git_buf_put in print_to_buffer_cb returned -1,
 	 * meaning a memory allocation failure, so just map to -1...
 	 */
-	if (error == GIT_EUSER)
+	if (error == GIT_EUSER) {
+		giterr_set_oom();
 		error = -1;
+	}
 
 	*string = git_buf_detach(&output);
 

src/diff_xdiff.c

@@ -28,25 +28,29 @@ static int git_xdiff_parse_hunk(git_diff_hunk *hunk, const char *header)
 {
 	/* expect something of the form "@@ -%d[,%d] +%d[,%d] @@" */
 	if (*header != '@')
-		return -1;
+		goto fail;
 	if (git_xdiff_scan_int(&header, &hunk->old_start) < 0)
-		return -1;
+		goto fail;
 	if (*header == ',') {
 		if (git_xdiff_scan_int(&header, &hunk->old_lines) < 0)
-			return -1;
+			goto fail;
 	} else
 		hunk->old_lines = 1;
 	if (git_xdiff_scan_int(&header, &hunk->new_start) < 0)
-		return -1;
+		goto fail;
 	if (*header == ',') {
 		if (git_xdiff_scan_int(&header, &hunk->new_lines) < 0)
-			return -1;
+			goto fail;
 	} else
 		hunk->new_lines = 1;
 	if (hunk->old_start < 0 || hunk->new_start < 0)
-		return -1;
+		goto fail;
 
 	return 0;
+
+fail:
+	giterr_set(GITERR_INVALID, "Malformed hunk header from xdiff");
+	return -1;
 }
 
 typedef struct {
@@ -123,7 +127,7 @@ static int git_xdiff_cb(void *priv, mmbuffer_t *bufs, int len)
 
 		if (output->hunk_cb != NULL &&
 			output->hunk_cb(delta, &info->hunk, output->payload))
-			output->error = GIT_EUSER;
+			return (output->error = giterr_user_cancel());
 
 		info->old_lineno = info->hunk.old_start;
 		info->new_lineno = info->hunk.new_start;
@@ -149,7 +153,7 @@ static int git_xdiff_cb(void *priv, mmbuffer_t *bufs, int len)
 		if (!output->error &&
 			output->data_cb != NULL &&
 			output->data_cb(delta, &info->hunk, &line, output->payload))
-			output->error = GIT_EUSER;
+			output->error = giterr_user_cancel();
 	}
 
 	if (len == 3 && !output->error) {
@@ -171,7 +175,7 @@ static int git_xdiff_cb(void *priv, mmbuffer_t *bufs, int len)
 		if (!output->error &&
 			output->data_cb != NULL &&
 			output->data_cb(delta, &info->hunk, &line, output->payload))
-			output->error = GIT_EUSER;
+			output->error = giterr_user_cancel();
 	}
 
 	return output->error;
@@ -219,11 +223,9 @@ void git_xdiff_init(git_xdiff_output *xo, const git_diff_options *opts)
 
 	xo->output.diff_cb = git_xdiff;
 
-	memset(&xo->config, 0, sizeof(xo->config));
 	xo->config.ctxlen = opts ? opts->context_lines : 3;
 	xo->config.interhunkctxlen = opts ? opts->interhunk_lines : 0;
 
-	memset(&xo->params, 0, sizeof(xo->params));
 	if (flags & GIT_DIFF_IGNORE_WHITESPACE)
 		xo->params.flags |= XDF_WHITESPACE_FLAGS;
 	if (flags & GIT_DIFF_IGNORE_WHITESPACE_CHANGE)
@@ -236,6 +238,5 @@ void git_xdiff_init(git_xdiff_output *xo, const git_diff_options *opts)
 	if (flags & GIT_DIFF_MINIMAL)
 		xo->params.flags |= XDF_NEED_MINIMAL;
 
-	memset(&xo->callback, 0, sizeof(xo->callback));
 	xo->callback.outf = git_xdiff_cb;
 }

src/errors.c

@@ -23,6 +23,7 @@ static void set_error(int error_class, char *string)
 {
 	git_error *error = &GIT_GLOBAL->error_t;
 
+	if (error->message != string)
 		git__free(error->message);
 
 	error->message = string;
@@ -103,8 +104,10 @@ int giterr_set_regex(const regex_t *regex, int error_code)
 
 void giterr_clear(void)
 {
+	if (GIT_GLOBAL->last_error != NULL) {
 		set_error(0, NULL);
 		GIT_GLOBAL->last_error = NULL;
+	}
 
 	errno = 0;
 #ifdef GIT_WIN32
@@ -134,3 +137,39 @@ const git_error *giterr_last(void)
 {
 	return GIT_GLOBAL->last_error;
 }
+
+int giterr_capture(git_error_state *state, int error_code)
+{
+	state->error_code = error_code;
+	if (error_code)
+		giterr_detach(&state->error_msg);
+	return error_code;
+}
+
+int giterr_restore(git_error_state *state)
+{
+	if (state && state->error_code && state->error_msg.message)
+		set_error(state->error_msg.klass, state->error_msg.message);
+	else
+		giterr_clear();
+
+	return state ? state->error_code : 0;
+}
+
+int giterr_system_last(void)
+{
+#ifdef GIT_WIN32
+	return GetLastError();
+#else
+	return errno;
+#endif
+}
+
+void giterr_system_set(int code)
+{
+#ifdef GIT_WIN32
+	SetLastError(code);
+#else
+	errno = code;
+#endif
+}

src/fileops.c

@@ -403,8 +403,8 @@ typedef struct {
 	const char *base;
 	size_t baselen;
 	uint32_t flags;
-	int error;
 	int depth;
+	git_error_state error;
 } futils__rmdir_data;
 
 #define FUTILS_MAX_DEPTH 100
@@ -447,8 +447,8 @@ static int futils__rm_first_parent(git_buf *path, const char *ceiling)
 
 static int futils__rmdir_recurs_foreach(void *opaque, git_buf *path)
 {
+	int error = 0;
 	futils__rmdir_data *data = opaque;
-	int error = data->error;
 	struct stat st;
 
 	if (data->depth > FUTILS_MAX_DEPTH)
@@ -474,13 +474,16 @@ static int futils__rmdir_recurs_foreach(void *opaque, git_buf *path)
 		data->depth++;
 
 		error = git_path_direach(path, 0, futils__rmdir_recurs_foreach, data);
-		if (error < 0)
-			return (error == GIT_EUSER) ? data->error : error;
+		if (error == GIT_EUSER)
+			return error;
 
 		data->depth--;
 
+		if (error < 0)
+			goto done;
+
 		if (data->depth == 0 && (data->flags & GIT_RMDIR_SKIP_ROOT) != 0)
-			return data->error;
+			goto done;
 
 		if ((error = p_rmdir(path->ptr)) < 0) {
 			if ((data->flags & GIT_RMDIR_SKIP_NONEMPTY) != 0 &&
@@ -499,35 +502,31 @@ static int futils__rmdir_recurs_foreach(void *opaque, git_buf *path)
 	else if ((data->flags & GIT_RMDIR_SKIP_NONEMPTY) == 0)
 		error = futils__error_cannot_rmdir(path->ptr, "still present");
 
-	data->error = error;
-	return error;
+done:
+	return giterr_capture(&data->error, error);
 }
 
 static int futils__rmdir_empty_parent(void *opaque, git_buf *path)
 {
 	futils__rmdir_data *data = opaque;
-	int error;
+	int error = 0;
 
 	if (git_buf_len(path) <= data->baselen)
-		return GIT_ITEROVER;
+		return giterr_capture(&data->error, GIT_ITEROVER);
 
-	error = p_rmdir(git_buf_cstr(path));
-
-	if (error) {
+	if (p_rmdir(git_buf_cstr(path)) < 0) {
 		int en = errno;
 
 		if (en == ENOENT || en == ENOTDIR) {
-			giterr_clear();
-			error = 0;
+			/* do nothing */
 		} else if (en == ENOTEMPTY || en == EEXIST || en == EBUSY) {
-			giterr_clear();
 			error = GIT_ITEROVER;
 		} else {
 			error = git_path_set_error(errno, git_buf_cstr(path), "rmdir");
 		}
 	}
 
-	return error;
+	return giterr_capture(&data->error, error);
 }
 
 int git_futils_rmdir_r(
@@ -535,12 +534,13 @@ int git_futils_rmdir_r(
 {
 	int error;
 	git_buf fullpath = GIT_BUF_INIT;
-	futils__rmdir_data data = { 0 };
+	futils__rmdir_data data;
 
 	/* build path and find "root" where we should start calling mkdir */
 	if (git_path_join_unrooted(&fullpath, path, base, NULL) < 0)
 		return -1;
 
+	memset(&data, 0, sizeof(data));
 	data.base    = base ? base : "";
 	data.baselen = base ? strlen(base) : 0;
 	data.flags   = flags;
@@ -548,13 +548,14 @@ int git_futils_rmdir_r(
 	error = futils__rmdir_recurs_foreach(&data, &fullpath);
 
 	/* remove now-empty parents if requested */
-	if (!error && (flags & GIT_RMDIR_EMPTY_PARENTS) != 0) {
+	if (!error && (flags & GIT_RMDIR_EMPTY_PARENTS) != 0)
 		error = git_path_walk_up(
 			&fullpath, base, futils__rmdir_empty_parent, &data);
 
+	if (error == GIT_EUSER)
+		error = giterr_restore(&data.error);
 	if (error == GIT_ITEROVER)
 		error = 0;
-	}
 
 	git_buf_free(&fullpath);
 
@@ -858,7 +859,7 @@ typedef struct {
 	uint32_t flags;
 	uint32_t mkdir_flags;
 	mode_t dirmode;
-	int error;
+	git_error_state error;
 } cp_r_info;
 
 #define GIT_CPDIR__MKDIR_DONE_FOR_TO_ROOT (1u << 10)
@@ -896,23 +897,21 @@ static int _cp_r_callback(void *ref, git_buf *from)
 		from->ptr[git_path_basename_offset(from)] == '.')
 		return 0;
 
-	if (git_buf_joinpath(
-			&info->to, info->to_root, from->ptr + info->from_prefix) < 0) {
-		error = -1;
-		goto exit;
-	}
+	if ((error = git_buf_joinpath(
+			&info->to, info->to_root, from->ptr + info->from_prefix)) < 0)
+		goto done;
 
 	if (!(error = git_path_lstat(info->to.ptr, &to_st)))
 		exists = true;
 	else if (error != GIT_ENOTFOUND)
-		goto exit;
+		goto done;
 	else {
 		giterr_clear();
 		error = 0;
 	}
 
 	if ((error = git_path_lstat(from->ptr, &from_st)) < 0)
-		goto exit;
+		goto done;
 
 	if (S_ISDIR(from_st.st_mode)) {
 		mode_t oldmode = info->dirmode;
@@ -928,15 +927,14 @@ static int _cp_r_callback(void *ref, git_buf *from)
 		/* recurse onto target directory */
 		if (!error && (!exists || S_ISDIR(to_st.st_mode))) {
 			error = git_path_direach(from, 0, _cp_r_callback, info);
-
 			if (error == GIT_EUSER)
-				error = info->error;
+				return error;
 		}
 
 		if (oldmode != 0)
 			info->dirmode = oldmode;
 
-		goto exit;
+		goto done;
 	}
 
 	if (exists) {
@@ -947,7 +945,7 @@ static int _cp_r_callback(void *ref, git_buf *from)
 			giterr_set(GITERR_OS, "Cannot overwrite existing file '%s'",
 				info->to.ptr);
 			error = -1;
-			goto exit;
+			goto done;
 		}
 	}
 
@@ -960,7 +958,7 @@ static int _cp_r_callback(void *ref, git_buf *from)
 	/* Make container directory on demand if needed */
 	if ((info->flags & GIT_CPDIR_CREATE_EMPTY_DIRS) == 0 &&
 		(error = _cp_r_mkdir(info, from)) < 0)
-		goto exit;
+		goto done;
 
 	/* make symlink or regular file */
 	if (S_ISLNK(from_st.st_mode))
@@ -974,9 +972,8 @@ static int _cp_r_callback(void *ref, git_buf *from)
 		error = git_futils_cp(from->ptr, info->to.ptr, usemode);
 	}
 
-exit:
-	info->error = error;
-	return error;
+done:
+	return giterr_capture(&info->error, error);
 }
 
 int git_futils_cp_r(
@@ -992,11 +989,11 @@ int git_futils_cp_r(
 	if (git_buf_joinpath(&path, from, "") < 0) /* ensure trailing slash */
 		return -1;
 
+	memset(&info, 0, sizeof(info));
 	info.to_root = to;
 	info.flags   = flags;
 	info.dirmode = dirmode;
 	info.from_prefix = path.size;
-	info.error = 0;
 	git_buf_init(&info.to, 0);
 
 	/* precalculate mkdir flags */
@@ -1019,7 +1016,7 @@ int git_futils_cp_r(
 	git_buf_free(&info.to);
 
 	if (error == GIT_EUSER)
-		error = info.error;
+		error = giterr_restore(&info.error);
 
 	return error;
 }

src/iterator.c

@@ -991,9 +991,8 @@ static int fs_iterator__expand_dir(fs_iterator *fi)
 		fi->base.start, fi->base.end, &ff->entries);
 
 	if (error < 0) {
-		git_error last_error = {0};
-
-		giterr_detach(&last_error);
+		git_error_state last_error = { 0 };
+		giterr_capture(&last_error, error);
 
 		/* these callbacks may clear the error message */
 		fs_iterator__free_frame(ff);
@@ -1001,12 +1000,7 @@ static int fs_iterator__expand_dir(fs_iterator *fi)
 		/* next time return value we skipped to */
 		fi->base.flags &= ~GIT_ITERATOR_FIRST_ACCESS;
 
-		if (last_error.message) {
-			giterr_set_str(last_error.klass, last_error.message);
-			free(last_error.message);
-		}
-
-		return error;
+		return giterr_restore(&last_error);
 	}
 
 	if (ff->entries.length == 0) {

src/odb_loose.c

@@ -696,7 +696,7 @@ struct foreach_state {
 	size_t dir_len;
 	git_odb_foreach_cb cb;
 	void *data;
-	int cb_error;
+	git_error_state cb_error;
 };
 
 GIT_INLINE(int) filename_to_oid(git_oid *oid, const char *ptr)
@@ -735,10 +735,8 @@ static int foreach_object_dir_cb(void *_state, git_buf *path)
 	if (filename_to_oid(&oid, path->ptr + state->dir_len) < 0)
 		return 0;
 
-	if (state->cb(&oid, state->data)) {
-		state->cb_error = GIT_EUSER;
-		return -1;
-	}
+	if (state->cb(&oid, state->data))
+		return giterr_user_cancel();
 
 	return 0;
 }
@@ -747,7 +745,9 @@ static int foreach_cb(void *_state, git_buf *path)
 {
 	struct foreach_state *state = (struct foreach_state *) _state;
 
-	return git_path_direach(path, 0, foreach_object_dir_cb, state);
+	return giterr_capture(
+		&state->cb_error,
+		git_path_direach(path, 0, foreach_object_dir_cb, state));
 }
 
 static int loose_backend__foreach(git_odb_backend *_backend, git_odb_foreach_cb cb, void *data)
@@ -762,7 +762,8 @@ static int loose_backend__foreach(git_odb_backend *_backend, git_odb_foreach_cb
 
 	objects_dir = backend->objects_dir;
 
-	git_buf_sets(&buf, objects_dir);
+	if (git_buf_sets(&buf, objects_dir) < 0)
+		return -1;
 	git_path_to_dir(&buf);
 
 	memset(&state, 0, sizeof(state));
@@ -772,9 +773,12 @@ static int loose_backend__foreach(git_odb_backend *_backend, git_odb_foreach_cb
 
 	error = git_path_direach(&buf, 0, foreach_cb, &state);
 
+	if (error == GIT_EUSER)
+		error = giterr_restore(&state.cb_error);
+
 	git_buf_free(&buf);
 
-	return state.cb_error ? state.cb_error : error;
+	return error;
 }
 
 static int loose_backend__stream_fwrite(git_odb_stream *_stream, const git_oid *oid)

src/odb_pack.c

@@ -190,31 +190,45 @@ static int packfile_sort__cb(const void *a_, const void *b_)
 }
 
 
+struct packfile_load_data {
+	struct pack_backend *backend;
+	git_error_state error;
+};
 
 static int packfile_load__cb(void *_data, git_buf *path)
 {
-	struct pack_backend *backend = (struct pack_backend *)_data;
+	struct packfile_load_data *data = _data;
+	struct pack_backend *backend = data->backend;
 	struct git_pack_file *pack;
+	const char *path_str = git_buf_cstr(path);
+	size_t i, cmp_len = git_buf_len(path);
 	int error;
-	size_t i;
 
-	if (git__suffixcmp(path->ptr, ".idx") != 0)
+	if (cmp_len <= strlen(".idx") || git__suffixcmp(path_str, ".idx") != 0)
 		return 0; /* not an index */
 
+	cmp_len -= strlen(".idx");
+
 	for (i = 0; i < backend->packs.length; ++i) {
 		struct git_pack_file *p = git_vector_get(&backend->packs, i);
-		if (memcmp(p->pack_name, git_buf_cstr(path), git_buf_len(path) - strlen(".idx")) == 0)
+
+		if (memcmp(p->pack_name, path_str, cmp_len) == 0)
 			return 0;
 	}
 
 	error = git_packfile_alloc(&pack, path->ptr);
-	if (error == GIT_ENOTFOUND)
+
 	/* ignore missing .pack file as git does */
+	if (error == GIT_ENOTFOUND) {
+		giterr_clear();
 		return 0;
-	else if (error < 0)
-		return error;
+	}
+
+	if (!error)
+		error = git_vector_insert(&backend->packs, pack);
+
+	return giterr_capture(&data->error, error);
 
-	return git_vector_insert(&backend->packs, pack);
 }
 
 static int pack_entry_find_inner(
@@ -314,32 +328,34 @@ static int pack_entry_find_prefix(
  * Implement the git_odb_backend API calls
  *
  ***********************************************************/
-static int pack_backend__refresh(git_odb_backend *_backend)
+static int pack_backend__refresh(git_odb_backend *backend)
 {
-	struct pack_backend *backend = (struct pack_backend *)_backend;
-
+	struct packfile_load_data data;
 	int error;
 	struct stat st;
 	git_buf path = GIT_BUF_INIT;
 
-	if (backend->pack_folder == NULL)
+	memset(&data, 0, sizeof(data));
+	data.backend = (struct pack_backend *)backend;
+
+	if (data.backend->pack_folder == NULL)
 		return 0;
 
-	if (p_stat(backend->pack_folder, &st) < 0 || !S_ISDIR(st.st_mode))
+	if (p_stat(data.backend->pack_folder, &st) < 0 || !S_ISDIR(st.st_mode))
 		return git_odb__error_notfound("failed to refresh packfiles", NULL);
 
-	git_buf_sets(&path, backend->pack_folder);
+	git_buf_sets(&path, data.backend->pack_folder);
 
 	/* reload all packs */
-	error = git_path_direach(&path, 0, packfile_load__cb, backend);
+	error = git_path_direach(&path, 0, packfile_load__cb, &data);
 
-	git_buf_free(&path);
+	if (error == GIT_EUSER)
+		error = giterr_restore(&data.error);
 
-	if (error < 0)
-		return -1;
+	git_buf_free(&path);
+	git_vector_sort(&data.backend->packs);
 
-	git_vector_sort(&backend->packs);
-	return 0;
+	return error;
 }
 
 static int pack_backend__read_header_internal(

src/path.c

@@ -434,7 +434,8 @@ int git_path_walk_up(
 	iter.asize = path->asize;
 
 	while (scan >= stop) {
-		error = cb(data, &iter);
+		if (cb(data, &iter))
+			error = giterr_user_cancel();
 		iter.ptr[scan] = oldc;
 		if (error < 0)
 			break;
@@ -528,7 +529,9 @@ bool git_path_is_empty_dir(const char *path)
 	if (!git_path_isdir(path))
 		return false;
 
-	if (!(error = git_buf_sets(&dir, path)))
+	if ((error = git_buf_sets(&dir, path)) != 0)
+		giterr_clear();
+	else
 		error = git_path_direach(&dir, 0, path_found_entry, NULL);
 
 	git_buf_free(&dir);
@@ -876,7 +879,7 @@ int git_path_direach(
 		git_buf_truncate(path, wd_len); /* restore path */
 
 		if (error) {
-			error = GIT_EUSER;
+			error = giterr_user_cancel();
 			break;
 		}
 	}

src/pool.c

@@ -217,7 +217,14 @@ char *git_pool_strdup(git_pool *pool, const char *str)
 
 char *git_pool_strdup_safe(git_pool *pool, const char *str)
 {
-	return str ? git_pool_strdup(pool, str) : NULL;
+	if (!str)
+		return NULL;
+	else {
+		char *result = git_pool_strdup(pool, str);
+		if (!result)
+			giterr_clear();
+		return result;
+	}
 }
 
 char *git_pool_strcat(git_pool *pool, const char *a, const char *b)

src/refdb_fs.c

@@ -264,9 +264,14 @@ done:
 	return error;
 }
 
-static int _dirent_loose_load(void *data, git_buf *full_path)
+struct packed_loadloose_data {
+	refdb_fs_backend *backend;
+	git_error_state error;
+};
+
+static int _dirent_loose_load(void *data_, git_buf *full_path)
 {
-	refdb_fs_backend *backend = (refdb_fs_backend *)data;
+	struct packed_loadloose_data *data = data_;
 	const char *file_path;
 
 	if (git__suffixcmp(full_path->ptr, ".lock") == 0)
@@ -274,11 +279,12 @@ static int _dirent_loose_load(void *data, git_buf *full_path)
 
 	if (git_path_isdir(full_path->ptr))
 		return git_path_direach(
-			full_path, backend->direach_flags, _dirent_loose_load, backend);
+			full_path, data->backend->direach_flags, _dirent_loose_load, data);
 
-	file_path = full_path->ptr + strlen(backend->path);
+	file_path = full_path->ptr + strlen(data->backend->path);
 
-	return loose_lookup_to_packfile(backend, file_path);
+	return giterr_capture(
+		&data->error, loose_lookup_to_packfile(data->backend, file_path));
 }
 
 /*
@@ -291,21 +297,28 @@ static int packed_loadloose(refdb_fs_backend *backend)
 {
 	int error;
 	git_buf refs_path = GIT_BUF_INIT;
+	struct packed_loadloose_data data;
 
 	if (git_buf_joinpath(&refs_path, backend->path, GIT_REFS_DIR) < 0)
 		return -1;
 
+	memset(&data, 0, sizeof(data));
+	data.backend = backend;
+
 	/*
 	 * Load all the loose files from disk into the Packfile table.
 	 * This will overwrite any old packed entries with their
 	 * updated loose versions
 	 */
 	error = git_path_direach(
-		&refs_path, backend->direach_flags, _dirent_loose_load, backend);
+		&refs_path, backend->direach_flags, _dirent_loose_load, &data);
 
 	git_buf_free(&refs_path);
 
-	return (error == GIT_EUSER) ? -1 : error;
+	if (error == GIT_EUSER)
+		error = giterr_restore(&data.error);
+
+	return error;
 }
 
 static int refdb_fs_backend__exists(

src/refs.c

@@ -471,7 +471,7 @@ int git_reference_rename(
 	if ((error = git_refdb_rename(out, ref->db, ref->name, new_name, force)) < 0)
 		return error;
 
-	/* Update HEAD it was poiting to the reference being renamed. */
+	/* Update HEAD it was pointing to the reference being renamed */
 	if (should_head_be_updated &&
 		(error = git_repository_set_head(ref->db->repo, new_name)) < 0) {
 		giterr_set(GITERR_REFERENCE, "Failed to update HEAD after renaming reference");

src/remote.c

@@ -251,13 +251,14 @@ int git_remote_create_inmemory(git_remote **out, git_repository *repo, const cha
 struct refspec_cb_data {
 	git_remote *remote;
 	int fetch;
+	git_error_state error;
 };
 
 static int refspec_cb(const git_config_entry *entry, void *payload)
 {
-	const struct refspec_cb_data *data = (struct refspec_cb_data *)payload;
-
-	return add_refspec(data->remote, entry->value, data->fetch);
+	struct refspec_cb_data *data = (struct refspec_cb_data *)payload;
+	return giterr_capture(
+		&data->error, add_refspec(data->remote, entry->value, data->fetch));
 }
 
 static int get_optional_config(
@@ -283,9 +284,6 @@ static int get_optional_config(
 		error = 0;
 	}
 
-	if (error < 0)
-		error = -1;
-
 	return error;
 }
 
@@ -296,7 +294,7 @@ int git_remote_load(git_remote **out, git_repository *repo, const char *name)
 	const char *val;
 	int error = 0;
 	git_config *config;
-	struct refspec_cb_data data;
+	struct refspec_cb_data data = { NULL };
 	bool optional_setting_found = false, found;
 
 	assert(out && repo && name);
@@ -363,6 +361,7 @@ int git_remote_load(git_remote **out, git_repository *repo, const char *name)
 
 	data.remote = remote;
 	data.fetch = true;
+
 	git_buf_clear(&buf);
 	git_buf_printf(&buf, "remote.%s.fetch", name);
 
@@ -388,6 +387,9 @@ int git_remote_load(git_remote **out, git_repository *repo, const char *name)
 cleanup:
 	git_buf_free(&buf);
 
+	if (error == GIT_EUSER)
+		error = giterr_restore(&data.error);
+
 	if (error < 0)
 		git_remote_free(remote);
 
@@ -1110,9 +1112,14 @@ void git_remote_free(git_remote *remote)
 	git__free(remote);
 }
 
+struct remote_list_data {
+	git_vector list;
+	git_error_state error;
+};
+
 static int remote_list_cb(const git_config_entry *entry, void *payload)
 {
-	git_vector *list = payload;
+	struct remote_list_data *data = payload;
 	const char *name = entry->name + strlen("remote.");
 	size_t namelen = strlen(name);
 	char *remote_name;
@@ -1123,47 +1130,50 @@ static int remote_list_cb(const git_config_entry *entry, void *payload)
 		remote_name = git__strndup(name, namelen - 4); /* strip ".url" */
 	else
 		remote_name = git__strndup(name, namelen - 8); /* strip ".pushurl" */
-	GITERR_CHECK_ALLOC(remote_name);
+	if (!remote_name)
+		return giterr_capture(&data->error, -1);
 
-	return git_vector_insert(list, remote_name);
+	return giterr_capture(
+		&data->error, git_vector_insert(&data->list, remote_name));
 }
 
 int git_remote_list(git_strarray *remotes_list, git_repository *repo)
 {
-	git_config *cfg;
-	git_vector list;
 	int error;
+	git_config *cfg;
+	struct remote_list_data data;
 
-	if (git_repository_config__weakptr(&cfg, repo) < 0)
-		return -1;
+	if ((error = git_repository_config__weakptr(&cfg, repo)) < 0)
+		return error;
 
-	if (git_vector_init(&list, 4, git__strcmp_cb) < 0)
-		return -1;
+	memset(&data, 0, sizeof(data));
+	if ((error = git_vector_init(&data.list, 4, git__strcmp_cb)) < 0)
+		return error;
 
 	error = git_config_foreach_match(
-		cfg, "^remote\\..*\\.(push)?url$", remote_list_cb, &list);
+		cfg, "^remote\\..*\\.(push)?url$", remote_list_cb, &data);
+
+	/* cb error is converted to GIT_EUSER by git_config_foreach */
+	if (error == GIT_EUSER)
+		error = giterr_restore(&data.error);
 
 	if (error < 0) {
 		size_t i;
 		char *elem;
 
-		git_vector_foreach(&list, i, elem) {
+		git_vector_foreach(&data.list, i, elem) {
 			git__free(elem);
 		}
 
-		git_vector_free(&list);
-
-		/* cb error is converted to GIT_EUSER by git_config_foreach */
-		if (error == GIT_EUSER)
-			error = -1;
+		git_vector_free(&data.list);
 
 		return error;
 	}
 
-	git_vector_uniq(&list, git__free);
+	git_vector_uniq(&data.list, git__free);
 
-	remotes_list->strings = (char **)list.contents;
-	remotes_list->count = list.length;
+	remotes_list->strings = (char **)data.list.contents;
+	remotes_list->count = data.list.length;
 
 	return 0;
 }
@@ -1250,11 +1260,11 @@ cleanup:
 	return error;
 }
 
-struct update_data
-{
+struct update_data {
 	git_config *config;
 	const char *old_remote_name;
 	const char *new_remote_name;
+	git_error_state error;
 };
 
 static int update_config_entries_cb(
@@ -1266,10 +1276,9 @@ static int update_config_entries_cb(
 	if (strcmp(entry->value, data->old_remote_name))
 		return 0;
 
-	return git_config_set_string(
-		data->config,
-		entry->name,
-		data->new_remote_name);
+	return giterr_capture(
+		&data->error, git_config_set_string(
+			data->config, entry->name, data->new_remote_name));
 }
 
 static int update_branch_remote_config_entry(
@@ -1277,20 +1286,22 @@ static int update_branch_remote_config_entry(
 	const char *old_name,
 	const char *new_name)
 {
-	git_config *config;
-	struct update_data data;
+	int error;
+	struct update_data data = { NULL };
 
-	if (git_repository_config__weakptr(&config, repo) < 0)
-		return -1;
+	if ((error = git_repository_config__weakptr(&data.config, repo)) < 0)
+		return error;
 
-	data.config = config;
 	data.old_remote_name = old_name;
 	data.new_remote_name = new_name;
 
-	return git_config_foreach_match(
-		config,
-		"branch\\..+\\.remote",
-		update_config_entries_cb, &data);
+	error = git_config_foreach_match(
+		data.config, "branch\\..+\\.remote", update_config_entries_cb, &data);
+
+	if (error == GIT_EUSER)
+		error = giterr_restore(&data.error);
+
+	return error;
 }
 
 static int rename_one_remote_reference(
@@ -1298,18 +1309,20 @@ static int rename_one_remote_reference(
 	const char *old_remote_name,
 	const char *new_remote_name)
 {
-	int error = -1;
+	int error;
 	git_buf new_name = GIT_BUF_INIT;
 
-	if (git_buf_printf(
+	error = git_buf_printf(
 		&new_name,
 		GIT_REFS_REMOTES_DIR "%s%s",
 		new_remote_name,
-		reference->name + strlen(GIT_REFS_REMOTES_DIR) + strlen(old_remote_name)) < 0)
-			return -1;
+		reference->name + strlen(GIT_REFS_REMOTES_DIR) + strlen(old_remote_name));
 
-	error = git_reference_rename(NULL, reference, git_buf_cstr(&new_name), 0);
+	if (!error) {
+		error = git_reference_rename(
+			NULL, reference, git_buf_cstr(&new_name), 0);
 		git_reference_free(reference);
+	}
 
 	git_buf_free(&new_name);
 	return error;
@@ -1320,12 +1333,12 @@ static int rename_remote_references(
 	const char *old_name,
 	const char *new_name)
 {
-	int error = -1;
+	int error;
 	git_reference *ref;
 	git_reference_iterator *iter;
 
-	if (git_reference_iterator_new(&iter, repo) < 0)
-		return -1;
+	if ((error = git_reference_iterator_new(&iter, repo)) < 0)
+		return error;
 
 	while ((error = git_reference_next(&ref, iter)) == 0) {
 		if (git__prefixcmp(ref->name, GIT_REFS_REMOTES_DIR)) {
@@ -1333,18 +1346,13 @@ static int rename_remote_references(
 			continue;
 		}
 
-		if ((error = rename_one_remote_reference(ref, old_name, new_name)) < 0) {
-			git_reference_iterator_free(iter);
-			return error;
-		}
+		if ((error = rename_one_remote_reference(ref, old_name, new_name)) < 0)
+			break;
 	}
 
 	git_reference_iterator_free(iter);
 
-	if (error == GIT_ITEROVER)
-		return 0;
-
-	return error;
+	return (error == GIT_ITEROVER) ? 0 : error;
 }
 
 static int rename_fetch_refspecs(

src/status.c

@@ -152,25 +152,32 @@ static git_status_t status_compute(
 	return st;
 }
 
+struct status_data {
+	git_status_list *status;
+	git_error_state err;
+};
+
 static int status_collect(
 	git_diff_delta *head2idx,
 	git_diff_delta *idx2wd,
 	void *payload)
 {
-	git_status_list *status = payload;
+	struct status_data *data = payload;
 	git_status_entry *status_entry;
 
-	if (!status_is_included(status, head2idx, idx2wd))
+	if (!status_is_included(data->status, head2idx, idx2wd))
 		return 0;
 
 	status_entry = git__malloc(sizeof(git_status_entry));
-	GITERR_CHECK_ALLOC(status_entry);
+	if (!status_entry)
+		return giterr_capture(&data->err, -1);
 
-	status_entry->status = status_compute(status, head2idx, idx2wd);
+	status_entry->status = status_compute(data->status, head2idx, idx2wd);
 	status_entry->head_to_index = head2idx;
 	status_entry->index_to_workdir = idx2wd;
 
-	return git_vector_insert(&status->paired, status_entry);
+	return giterr_capture(
+		&data->err, git_vector_insert(&data->status->paired, status_entry));
 }
 
 GIT_INLINE(int) status_entry_cmp_base(
@@ -314,9 +321,18 @@ int git_status_list_new(
 			goto done;
 	}
 
-	if ((error = git_diff__paired_foreach(
-			status->head2idx, status->idx2wd, status_collect, status)) < 0)
+	{
+		struct status_data data = { 0 };
+		data.status = status;
+
+		error = git_diff__paired_foreach(
+			status->head2idx, status->idx2wd, status_collect, &data);
+
+		if (error == GIT_EUSER)
+			error = giterr_restore(&data.err);
+		if (error < 0)
 			goto done;
+	}
 
 	if (flags & GIT_STATUS_OPT_SORT_CASE_SENSITIVELY)
 		git_vector_set_cmp(&status->paired, status_entry_cmp);

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;
+	}
 
 	/* 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);
 

tests/config/rename.c

+#include "clar_libgit2.h"
+#include "config.h"
+
+static git_repository *g_repo = NULL;
+static git_config *g_config = NULL;
+
+void test_config_rename__initialize(void)
+{
+    g_repo = cl_git_sandbox_init("testrepo.git");
+	cl_git_pass(git_repository_config(&g_config, g_repo));
+}
+
+void test_config_rename__cleanup(void)
+{
+	git_config_free(g_config);
+	g_config = NULL;
+
+	cl_git_sandbox_cleanup();
+	g_repo = NULL;
+}
+
+void test_config_rename__can_rename(void)
+{
+	const git_config_entry *ce;
+
+	cl_git_pass(git_config_get_entry(
+		&ce, g_config, "branch.track-local.remote"));
+	cl_assert_equal_s(".", ce->value);
+
+	cl_git_fail(git_config_get_entry(
+		&ce, g_config, "branch.local-track.remote"));
+
+	cl_git_pass(git_config_rename_section(
+		g_repo, "branch.track-local", "branch.local-track"));
+
+	cl_git_pass(git_config_get_entry(
+		&ce, g_config, "branch.local-track.remote"));
+	cl_assert_equal_s(".", ce->value);
+
+	cl_git_fail(git_config_get_entry(
+		&ce, g_config, "branch.track-local.remote"));
+}
+
+void test_config_rename__prevent_overwrite(void)
+{
+	const git_config_entry *ce;
+	const git_error *err;
+
+	cl_git_pass(git_config_set_string(
+		g_config, "branch.local-track.remote", "yellow"));
+
+	cl_git_pass(git_config_get_entry(
+		&ce, g_config, "branch.local-track.remote"));
+	cl_assert_equal_s("yellow", ce->value);
+
+	cl_git_pass(git_config_rename_section(
+		g_repo, "branch.track-local", "branch.local-track"));
+
+	cl_git_pass(git_config_get_entry(
+		&ce, g_config, "branch.local-track.remote"));
+	cl_assert_equal_s(".", ce->value);
+
+//	cl_assert((err = giterr_last()) != NULL);
+//	cl_assert(err->message != NULL);
+}
+
+static void assert_invalid_config_section_name(
+	git_repository *repo, const char *name)
+{
+	cl_git_fail_with(
+		git_config_rename_section(repo, "branch.remoteless", name),
+		GIT_EINVALIDSPEC);
+}
+
+void test_config_rename__require_a_valid_new_name(void)
+{
+	assert_invalid_config_section_name(g_repo, "");
+	assert_invalid_config_section_name(g_repo, "bra\nch");
+	assert_invalid_config_section_name(g_repo, "branc#");
+	assert_invalid_config_section_name(g_repo, "bra\nch.duh");
+	assert_invalid_config_section_name(g_repo, "branc#.duh");
+}

tests/config/validkeyname.c

@@ -46,23 +46,3 @@ void test_config_validkeyname__accessing_requires_a_valid_name(void)
 	assert_invalid_config_key_name("dif.dir\nstat.lines");
 	assert_invalid_config_key_name("dif.dirstat.li\nes");
 }
-
-static void assert_invalid_config_section_name(git_repository *repo, const char *name)
-{
-	cl_git_fail_with(git_config_rename_section(repo, "branch.remoteless", name), GIT_EINVALIDSPEC);
-}
-
-void test_config_validkeyname__renaming_a_section_requires_a_valid_name(void)
-{
-	git_repository *repo;
-
-	cl_git_pass(git_repository_open(&repo, cl_fixture("testrepo.git")));
-
-	assert_invalid_config_section_name(repo, "");
-	assert_invalid_config_section_name(repo, "bra\nch");
-	assert_invalid_config_section_name(repo, "branc#");
-	assert_invalid_config_section_name(repo, "bra\nch.duh");
-	assert_invalid_config_section_name(repo, "branc#.duh");
-
-	git_repository_free(repo);
-}

tests/config/write.c

@@ -303,3 +303,4 @@ void test_config_write__updating_a_locked_config_file_returns_ELOCKED(void)
 
 	git_config_free(cfg);
 }
+

tests/refs/branches/move.c

@@ -66,12 +66,54 @@ void test_refs_branches_move__can_move_a_local_branch_to_a_partially_colliding_n
 void test_refs_branches_move__can_not_move_a_branch_if_its_destination_name_collide_with_an_existing_one(void)
 {
 	git_reference *original_ref, *new_ref;
+	git_config *config;
+	const git_config_entry *ce;
+	char *original_remote, *original_merge;
+
+	cl_git_pass(git_repository_config(&config, repo));
+
+	cl_git_pass(git_config_get_entry(&ce, config, "branch.master.remote"));
+	original_remote = strdup(ce->value);
+	cl_git_pass(git_config_get_entry(&ce, config, "branch.master.merge"));
+	original_merge  = strdup(ce->value);
+
 
 	cl_git_pass(git_reference_lookup(&original_ref, repo, "refs/heads/br2"));
 
-	cl_assert_equal_i(GIT_EEXISTS, git_branch_move(&new_ref, original_ref, "master", 0));
+	cl_assert_equal_i(GIT_EEXISTS,
+		git_branch_move(&new_ref, original_ref, "master", 0));
+
+	cl_assert(giterr_last()->message != NULL);
+	cl_git_pass(git_config_get_entry(&ce, config, "branch.master.remote"));
+	cl_assert_equal_s(original_remote, ce->value);
+	cl_git_pass(git_config_get_entry(&ce, config, "branch.master.merge"));
+	cl_assert_equal_s(original_merge,  ce->value);
+
+
+	cl_assert_equal_i(GIT_EEXISTS,
+		git_branch_move(&new_ref, original_ref, "cannot-fetch", 0));
+
+	cl_assert(giterr_last()->message != NULL);
+	cl_git_pass(git_config_get_entry(&ce, config, "branch.master.remote"));
+	cl_assert_equal_s(original_remote, ce->value);
+	cl_git_pass(git_config_get_entry(&ce, config, "branch.master.merge"));
+	cl_assert_equal_s(original_merge,  ce->value);
+
+	git_reference_free(original_ref);
+	cl_git_pass(git_reference_lookup(&original_ref, repo, "refs/heads/track-local"));
+
+	cl_assert_equal_i(GIT_EEXISTS,
+		git_branch_move(&new_ref, original_ref, "master", 0));
+
+	cl_assert(giterr_last()->message != NULL);
+	cl_git_pass(git_config_get_entry(&ce, config, "branch.master.remote"));
+	cl_assert_equal_s(original_remote, ce->value);
+	cl_git_pass(git_config_get_entry(&ce, config, "branch.master.merge"));
+	cl_assert_equal_s(original_merge,  ce->value);
 
+	free(original_remote); free(original_merge);
 	git_reference_free(original_ref);
+	git_config_free(config);
 }
 
 void test_refs_branches_move__moving_a_branch_with_an_invalid_name_returns_EINVALIDSPEC(void)