Merge pull request #1526 from arrbee/cleanup-error-return-without-msg

Make sure error messages are set for most error returns

src/branch.c

@@ -185,7 +185,7 @@ int git_branch_move(
 		git_buf_cstr(&old_config_section),
 		git_buf_cstr(&new_config_section))) < 0)
 		goto done;
-	
+
 	if ((error = git_reference_rename(out, branch, git_buf_cstr(&new_reference_name), force)) < 0)
 		goto done;
 
@@ -276,6 +276,8 @@ int git_branch_upstream__name(
 			goto cleanup;
 
 	if (!*remote_name || !*merge_name) {
+		giterr_set(GITERR_REFERENCE,
+			"branch '%s' does not have an upstream", canonical_branch_name);
 		error = GIT_ENOTFOUND;
 		goto cleanup;
 	}
@@ -342,6 +344,9 @@ static int remote_name(git_buf *buf, git_repository *repo, const char *canonical
 				remote_name = remote_list.strings[i];
 			} else {
 				git_remote_free(remote);
+
+				giterr_set(GITERR_REFERENCE,
+					"Reference '%s' is ambiguous", canonical_branch_name);
 				error = GIT_EAMBIGUOUS;
 				goto cleanup;
 			}
@@ -354,6 +359,8 @@ static int remote_name(git_buf *buf, git_repository *repo, const char *canonical
 		git_buf_clear(buf);
 		error = git_buf_puts(buf, remote_name);
 	} else {
+		giterr_set(GITERR_REFERENCE,
+			"Could not determine remote for '%s'", canonical_branch_name);
 		error = GIT_ENOTFOUND;
 	}
 
@@ -490,8 +497,11 @@ int git_branch_set_upstream(git_reference *branch, const char *upstream_name)
 		local = 1;
 	else if (git_branch_lookup(&upstream, repo, upstream_name, GIT_BRANCH_REMOTE) == 0)
 		local = 0;
-	else
+	else {
+		giterr_set(GITERR_REFERENCE,
+			"Cannot set upstream for branch '%s'", shortname);
 		return GIT_ENOTFOUND;
+	}
 
 	/*
 	 * If it's local, the remote is "." and the branch name is

src/clone.c

@@ -397,18 +397,6 @@ on_error:
 }
 
 
-static bool path_is_okay(const char *path)
-{
-	/* The path must either not exist, or be an empty directory */
-	if (!git_path_exists(path)) return true;
-	if (!git_path_is_empty_dir(path)) {
-		giterr_set(GITERR_INVALID,
-					  "'%s' exists and is not an empty directory", path);
-		return false;
-	}
-	return true;
-}
-
 static bool should_checkout(
 	git_repository *repo,
 	bool is_bare,
@@ -454,7 +442,10 @@ int git_clone(
 	normalize_options(&normOptions, options);
 	GITERR_CHECK_VERSION(&normOptions, GIT_CLONE_OPTIONS_VERSION, "git_clone_options");
 
-	if (!path_is_okay(local_path)) {
+	/* Only clone to a new directory or an empty directory */
+	if (git_path_exists(local_path) && !git_path_is_empty_dir(local_path)) {
+		giterr_set(GITERR_INVALID,
+			"'%s' exists and is not an empty directory", local_path);
 		return GIT_ERROR;
 	}
 

src/config.c

@@ -344,6 +344,13 @@ int git_config_delete_entry(git_config *cfg, const char *name)
  * Setters
  **************/
 
+static int config_error_nofiles(const char *name)
+{
+	giterr_set(GITERR_CONFIG,
+		"Cannot set value for '%s' when no config files exist", name);
+	return GIT_ENOTFOUND;
+}
+
 int git_config_set_int64(git_config *cfg, const char *name, int64_t value)
 {
 	char str_value[32]; /* All numbers should fit in here */
@@ -373,12 +380,9 @@ int git_config_set_string(git_config *cfg, const char *name, const char *value)
 	}
 
 	internal = git_vector_get(&cfg->files, 0);
-	if (!internal) {
+	if (!internal)
 		/* Should we auto-vivify .git/config? Tricky from this location */
-		giterr_set(GITERR_CONFIG, "Cannot set value when no config files exist");
-		return GIT_ENOTFOUND;
-	}
-
+		return config_error_nofiles(name);
 	file = internal->file;
 
 	error = file->set(file, name, value);
@@ -442,6 +446,12 @@ static int get_string_at_file(const char **out, const git_config_backend *file,
 	return res;
 }
 
+static int config_error_notfound(const char *name)
+{
+	giterr_set(GITERR_CONFIG, "Config value '%s' was not found", name);
+	return GIT_ENOTFOUND;
+}
+
 static int get_string(const char **out, const git_config *cfg, const char *name)
 {
 	file_internal *internal;
@@ -454,7 +464,7 @@ static int get_string(const char **out, const git_config *cfg, const char *name)
 			return res;
 	}
 
-	return GIT_ENOTFOUND;
+	return config_error_notfound(name);
 }
 
 int git_config_get_bool(int *out, const git_config *cfg, const char *name)
@@ -494,7 +504,7 @@ int git_config_get_entry(const git_config_entry **out, const git_config *cfg, co
 			return ret;
 	}
 
-	return GIT_ENOTFOUND;
+	return config_error_notfound(name);
 }
 
 int git_config_get_multivar(const git_config *cfg, const char *name, const char *regexp,
@@ -517,7 +527,7 @@ int git_config_get_multivar(const git_config *cfg, const char *name, const char
 			return ret;
 	}
 
-	return 0;
+	return (ret == GIT_ENOTFOUND) ? config_error_notfound(name) : 0;
 }
 
 int git_config_set_multivar(git_config *cfg, const char *name, const char *regexp, const char *value)
@@ -526,6 +536,8 @@ int git_config_set_multivar(git_config *cfg, const char *name, const char *regex
 	file_internal *internal;
 
 	internal = git_vector_get(&cfg->files, 0);
+	if (!internal)
+		return config_error_nofiles(name);
 	file = internal->file;
 
 	return file->set_multivar(file, name, regexp, value);

src/diff_output.c

@@ -1611,6 +1611,12 @@ int git_diff_patch_line_stats(
 	return 0;
 }
 
+static int diff_error_outofrange(const char *thing)
+{
+	giterr_set(GITERR_INVALID, "Diff patch %s index out of range", thing);
+	return GIT_ENOTFOUND;
+}
+
 int git_diff_patch_get_hunk(
 	const git_diff_range **range,
 	const char **header,
@@ -1628,7 +1634,8 @@ int git_diff_patch_get_hunk(
 		if (header) *header = NULL;
 		if (header_len) *header_len = 0;
 		if (lines_in_hunk) *lines_in_hunk = 0;
-		return GIT_ENOTFOUND;
+
+		return diff_error_outofrange("hunk");
 	}
 
 	hunk = &patch->hunks[hunk_idx];
@@ -1648,7 +1655,7 @@ int git_diff_patch_num_lines_in_hunk(
 	assert(patch);
 
 	if (hunk_idx >= patch->hunks_size)
-		return GIT_ENOTFOUND;
+		return diff_error_outofrange("hunk");
 	else
 		return (int)patch->hunks[hunk_idx].line_count;
 }
@@ -1665,15 +1672,20 @@ int git_diff_patch_get_line_in_hunk(
 {
 	diff_patch_hunk *hunk;
 	diff_patch_line *line;
+	const char *thing;
 
 	assert(patch);
 
-	if (hunk_idx >= patch->hunks_size)
+	if (hunk_idx >= patch->hunks_size) {
+		thing = "hunk";
 		goto notfound;
+	}
 	hunk = &patch->hunks[hunk_idx];
 
-	if (line_of_hunk >= hunk->line_count)
+	if (line_of_hunk >= hunk->line_count) {
+		thing = "link";
 		goto notfound;
+	}
 
 	line = &patch->lines[hunk->line_start + line_of_hunk];
 
@@ -1692,7 +1704,7 @@ notfound:
 	if (old_lineno) *old_lineno = -1;
 	if (new_lineno) *new_lineno = -1;
 
-	return GIT_ENOTFOUND;
+	return diff_error_outofrange(thing);
 }
 
 static int print_to_buffer_cb(

src/fileops.c

@@ -988,8 +988,10 @@ int git_futils_filestamp_check(
 	if (stamp == NULL)
 		return 1;
 
-	if (p_stat(path, &st) < 0)
+	if (p_stat(path, &st) < 0) {
+		giterr_set(GITERR_OS, "Could not stat '%s'", path);
 		return GIT_ENOTFOUND;
+	}
 
 	if (stamp->mtime == (git_time_t)st.st_mtime &&
 		stamp->size  == (git_off_t)st.st_size   &&

src/index.c

@@ -547,8 +547,10 @@ const git_index_entry *git_index_get_bypath(
 
 	git_vector_sort(&index->entries);
 
-	if (index_find(&pos, index, path, stage) < 0)
+	if (index_find(&pos, index, path, stage) < 0) {
+		giterr_set(GITERR_INDEX, "Index does not contain %s", path);
 		return NULL;
+	}
 
 	return git_index_get_byindex(index, pos);
 }
@@ -830,8 +832,11 @@ int git_index_remove(git_index *index, const char *path, int stage)
 
 	git_vector_sort(&index->entries);
 
-	if (index_find(&position, index, path, stage) < 0)
+	if (index_find(&position, index, path, stage) < 0) {
+		giterr_set(GITERR_INDEX, "Index does not contain %s at stage %d",
+			path, stage);
 		return GIT_ENOTFOUND;
+	}
 
 	entry = git_vector_get(&index->entries, position);
 	if (entry != NULL)

src/merge.c

@@ -127,6 +127,7 @@ int git_merge_base_many(git_oid *out, git_repository *repo, const git_oid input_
 		goto cleanup;
 
 	if (!result) {
+		giterr_set(GITERR_MERGE, "No merge base found");
 		error = GIT_ENOTFOUND;
 		goto cleanup;
 	}
@@ -172,7 +173,7 @@ int git_merge_base(git_oid *out, git_repository *repo, const git_oid *one, const
 
 	if (!result) {
 		git_revwalk_free(walk);
-		giterr_clear();
+		giterr_set(GITERR_MERGE, "No merge base found");
 		return GIT_ENOTFOUND;
 	}
 

src/notes.c

@@ -13,6 +13,12 @@
 #include "iterator.h"
 #include "signature.h"
 
+static int note_error_notfound(void)
+{
+	giterr_set(GITERR_INVALID, "Note could not be found");
+	return GIT_ENOTFOUND;
+}
+
 static int find_subtree_in_current_level(
 	git_tree **out,
 	git_repository *repo,
@@ -26,7 +32,7 @@ static int find_subtree_in_current_level(
 	*out = NULL;
 
 	if (parent == NULL)
-		return GIT_ENOTFOUND;
+		return note_error_notfound();
 
 	for (i = 0; i < git_tree_entrycount(parent); i++) {
 		entry = git_tree_entry_byindex(parent, i);
@@ -44,7 +50,7 @@ static int find_subtree_in_current_level(
 			return GIT_EEXISTS;
 	}
 
-	return GIT_ENOTFOUND;
+	return note_error_notfound();
 }
 
 static int find_subtree_r(git_tree **out, git_tree *root,
@@ -56,9 +62,8 @@ static int find_subtree_r(git_tree **out, git_tree *root,
 	*out = NULL;
 
 	error = find_subtree_in_current_level(&subtree, repo, root, target, *fanout);
-	if (error == GIT_EEXISTS) {
+	if (error == GIT_EEXISTS)
 		return git_tree_lookup(out, repo, git_tree_id(root));
-	}
 
 	if (error < 0)
 		return error;
@@ -85,7 +90,8 @@ static int find_blob(git_oid *blob, git_tree *tree, const char *target)
 			return 0;
 		}
 	}
-	return GIT_ENOTFOUND;
+
+	return note_error_notfound();
 }
 
 static int tree_write(
@@ -316,8 +322,8 @@ static int note_new(git_note **out, git_oid *note_oid, git_blob *blob)
 	return 0;
 }
 
-static int note_lookup(git_note **out, git_repository *repo,
-		       git_tree *tree, const char *target)
+static int note_lookup(
+	git_note **out, git_repository *repo, git_tree *tree, const char *target)
 {
 	int error, fanout = 0;
 	git_oid oid;
@@ -382,6 +388,7 @@ static int note_get_default_ref(const char **out, git_repository *repo)
 
 	ret = git_config_get_string(out, cfg, "core.notesRef");
 	if (ret == GIT_ENOTFOUND) {
+		giterr_clear();
 		*out = GIT_NOTES_DEFAULT_REF;
 		return 0;
 	}
@@ -432,12 +439,10 @@ int git_note_read(git_note **out, git_repository *repo,
 	target = git_oid_allocfmt(oid);
 	GITERR_CHECK_ALLOC(target);
 
-	if ((error = retrieve_note_tree_and_commit(&tree, &commit, repo, &notes_ref)) < 0)
-		goto cleanup;
-
-	error = note_lookup(out, repo, tree, target);
+	if (!(error = retrieve_note_tree_and_commit(
+			&tree, &commit, repo, &notes_ref)))
+		error = note_lookup(out, repo, tree, target);
 
-cleanup:
 	git__free(target);
 	git_tree_free(tree);
 	git_commit_free(commit);
@@ -489,13 +494,11 @@ int git_note_remove(git_repository *repo, const char *notes_ref,
 	target = git_oid_allocfmt(oid);
 	GITERR_CHECK_ALLOC(target);
 
-	if ((error = retrieve_note_tree_and_commit(&tree, &commit, repo, &notes_ref)) < 0)
-		goto cleanup;
-
-	error = note_remove(repo, author, committer, notes_ref,
-			    tree, target, &commit);
+	if (!(error = retrieve_note_tree_and_commit(
+			&tree, &commit, repo, &notes_ref)))
+		error = note_remove(
+			repo, author, committer, notes_ref, tree, target, &commit);
 
-cleanup:
 	git__free(target);
 	git_commit_free(commit);
 	git_tree_free(tree);
@@ -533,7 +536,7 @@ static int process_entry_path(
 	const char* entry_path,
 	git_oid *annotated_object_id)
 {
-	int error = -1;
+	int error = 0;
 	size_t i = 0, j = 0, len;
 	git_buf buf = GIT_BUF_INIT;
 

src/object.c

@@ -122,8 +122,10 @@ int git_object_lookup_prefix(
 
 	assert(repo && object_out && id);
 
-	if (len < GIT_OID_MINPREFIXLEN)
+	if (len < GIT_OID_MINPREFIXLEN) {
+		giterr_set(GITERR_OBJECT, "Ambiguous lookup - OID prefix is too short");
 		return GIT_EAMBIGUOUS;
+	}
 
 	error = git_repository_odb__weakptr(&odb, repo);
 	if (error < 0)
@@ -311,18 +313,17 @@ int git_object_peel(
 	git_object *source, *deref = NULL;
 	int error;
 
-	if (target_type != GIT_OBJ_TAG && 
-		target_type != GIT_OBJ_COMMIT && 
-		target_type != GIT_OBJ_TREE && 
-		target_type != GIT_OBJ_BLOB && 
-		target_type != GIT_OBJ_ANY)
-			return GIT_EINVALIDSPEC;
-
 	assert(object && peeled);
 
 	if (git_object_type(object) == target_type)
 		return git_object_dup(peeled, (git_object *)object);
 
+	assert(target_type == GIT_OBJ_TAG ||
+		target_type == GIT_OBJ_COMMIT ||
+		target_type == GIT_OBJ_TREE ||
+		target_type == GIT_OBJ_BLOB ||
+		target_type == GIT_OBJ_ANY);
+
 	source = (git_object *)object;
 
 	while (!(error = dereference_object(&deref, source))) {

src/odb.c

@@ -424,6 +424,14 @@ size_t git_odb_num_backends(git_odb *odb)
 	return odb->backends.length;
 }
 
+static int git_odb__error_unsupported_in_backend(const char *action)
+{
+	giterr_set(GITERR_ODB,
+		"Cannot %s - unsupported in the loaded odb backends", action);
+	return -1;
+}
+
+
 int git_odb_get_backend(git_odb_backend **out, git_odb *odb, size_t pos)
 {
 	backend_internal *internal;
@@ -436,6 +444,7 @@ int git_odb_get_backend(git_odb_backend **out, git_odb *odb, size_t pos)
 		return 0;
 	}
 
+	giterr_set(GITERR_ODB, "No ODB backend loaded at index " PRIuZ, pos);
 	return GIT_ENOTFOUND;
 }
 
@@ -472,7 +481,7 @@ static int add_default_backends(
 			return 0;
 	}
 #endif
-	
+
 	/* add the loose object backend */
 	if (git_odb_backend_loose(&loose, objects_dir, -1, 0) < 0 ||
 		add_backend_internal(db, loose, GIT_LOOSE_PRIORITY, as_alternates, inode) < 0)
@@ -495,9 +504,8 @@ static int load_alternates(git_odb *odb, const char *objects_dir, int alternate_
 	int result = 0;
 
 	/* Git reports an error, we just ignore anything deeper */
-	if (alternate_depth > GIT_ALTERNATES_MAX_DEPTH) {
+	if (alternate_depth > GIT_ALTERNATES_MAX_DEPTH)
 		return 0;
-	}
 
 	if (git_buf_joinpath(&alternates_path, objects_dir, GIT_ALTERNATES_FILE) < 0)
 		return -1;
@@ -687,7 +695,7 @@ int git_odb__read_header_or_object(
 
 int git_odb_read(git_odb_object **out, git_odb *db, const git_oid *id)
 {
-	size_t i;
+	size_t i, reads = 0;
 	int error;
 	bool refreshed = false;
 	git_rawobj raw;
@@ -695,11 +703,6 @@ int git_odb_read(git_odb_object **out, git_odb *db, const git_oid *id)
 
 	assert(out && db && id);
 
-	if (db->backends.length == 0) {
-		giterr_set(GITERR_ODB, "Failed to lookup object: no backends loaded");
-		return GIT_ENOTFOUND;
-	}
-
 	*out = git_cache_get_raw(odb_cache(db), id);
 	if (*out != NULL)
 		return 0;
@@ -711,8 +714,10 @@ attempt_lookup:
 		backend_internal *internal = git_vector_get(&db->backends, i);
 		git_odb_backend *b = internal->backend;
 
-		if (b->read != NULL)
+		if (b->read != NULL) {
+			++reads;
 			error = b->read(&raw.data, &raw.len, &raw.type, b, id);
+		}
 	}
 
 	if (error == GIT_ENOTFOUND && !refreshed) {
@@ -723,8 +728,11 @@ attempt_lookup:
 		goto attempt_lookup;
 	}
 
-	if (error && error != GIT_PASSTHROUGH)
+	if (error && error != GIT_PASSTHROUGH) {
+		if (!reads)
+			return git_odb__error_notfound("no match for id", id);
 		return error;
+	}
 
 	if ((object = odb_object__alloc(id, &raw)) == NULL)
 		return -1;
@@ -844,10 +852,10 @@ int git_odb_write(
 	if (!error || error == GIT_PASSTHROUGH)
 		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 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)) != 0)
 		return error;
 
@@ -861,7 +869,7 @@ int git_odb_write(
 int git_odb_open_wstream(
 	git_odb_stream **stream, git_odb *db, size_t size, git_otype type)
 {
-	size_t i;
+	size_t i, writes = 0;
 	int error = GIT_ERROR;
 
 	assert(stream && db);
@@ -874,21 +882,26 @@ int git_odb_open_wstream(
 		if (internal->is_alternate)
 			continue;
 
-		if (b->writestream != NULL)
+		if (b->writestream != NULL) {
+			++writes;
 			error = b->writestream(stream, b, size, type);
-		else if (b->write != NULL)
+		} else if (b->write != NULL) {
+			++writes;
 			error = init_fake_wstream(stream, b, size, type);
+		}
 	}
 
 	if (error == GIT_PASSTHROUGH)
 		error = 0;
+	if (error < 0 && !writes)
+		error = git_odb__error_unsupported_in_backend("write object");
 
 	return error;
 }
 
 int git_odb_open_rstream(git_odb_stream **stream, git_odb *db, const git_oid *oid)
 {
-	size_t i;
+	size_t i, reads = 0;
 	int error = GIT_ERROR;
 
 	assert(stream && db);
@@ -897,19 +910,23 @@ int git_odb_open_rstream(git_odb_stream **stream, git_odb *db, const git_oid *oi
 		backend_internal *internal = git_vector_get(&db->backends, i);
 		git_odb_backend *b = internal->backend;
 
-		if (b->readstream != NULL)
+		if (b->readstream != NULL) {
+			++reads;
 			error = b->readstream(stream, b, oid);
+		}
 	}
 
 	if (error == GIT_PASSTHROUGH)
 		error = 0;
+	if (error < 0 && !reads)
+		error = git_odb__error_unsupported_in_backend("read object streamed");
 
 	return error;
 }
 
 int git_odb_write_pack(struct git_odb_writepack **out, git_odb *db, git_transfer_progress_callback progress_cb, void *progress_payload)
 {
-	size_t i;
+	size_t i, writes = 0;
 	int error = GIT_ERROR;
 
 	assert(out && db);
@@ -922,12 +939,16 @@ int git_odb_write_pack(struct git_odb_writepack **out, git_odb *db, git_transfer
 		if (internal->is_alternate)
 			continue;
 
-		if (b->writepack != NULL)
+		if (b->writepack != NULL) {
+			++writes;
 			error = b->writepack(out, b, progress_cb, progress_payload);
+		}
 	}
 
 	if (error == GIT_PASSTHROUGH)
 		error = 0;
+	if (error < 0 && !writes)
+		error = git_odb__error_unsupported_in_backend("write pack");
 
 	return error;
 }

src/push.c

@@ -303,7 +303,7 @@ static int revwalk(git_vector *commits, git_push *push)
 				continue;
 
 			if (!git_odb_exists(push->repo->_odb, &spec->roid)) {
-				giterr_clear();
+				giterr_set(GITERR_REFERENCE, "Cannot push missing reference");
 				error = GIT_ENONFASTFORWARD;
 				goto on_error;
 			}
@@ -313,7 +313,8 @@ static int revwalk(git_vector *commits, git_push *push)
 
 			if (error == GIT_ENOTFOUND ||
 				(!error && !git_oid_equal(&base, &spec->roid))) {
-				giterr_clear();
+				giterr_set(GITERR_REFERENCE,
+					"Cannot push non-fastforwardable reference");
 				error = GIT_ENONFASTFORWARD;
 				goto on_error;
 			}
@@ -333,12 +334,13 @@ static int revwalk(git_vector *commits, git_push *push)
 
 	while ((error = git_revwalk_next(&oid, rw)) == 0) {
 		git_oid *o = git__malloc(GIT_OID_RAWSZ);
-		GITERR_CHECK_ALLOC(o);
-		git_oid_cpy(o, &oid);
-		if (git_vector_insert(commits, o) < 0) {
+		if (!o) {
 			error = -1;
 			goto on_error;
 		}
+		git_oid_cpy(o, &oid);
+		if ((error = git_vector_insert(commits, o)) < 0)
+			goto on_error;
 	}
 
 on_error:
@@ -519,7 +521,7 @@ static int calculate_work(git_push *push)
 			/* This is a create or update.  Local ref must exist. */
 			if (git_reference_name_to_id(
 					&spec->loid, push->repo, spec->lref) < 0) {
-				giterr_set(GIT_ENOTFOUND, "No such reference '%s'", spec->lref);
+				giterr_set(GITERR_REFERENCE, "No such reference '%s'", spec->lref);
 				return -1;
 			}
 		}

src/reflog.c

@@ -483,8 +483,10 @@ int git_reflog_drop(
 
 	entry = (git_reflog_entry *)git_reflog_entry_byindex(reflog, idx);
 
-	if (entry == NULL)
+	if (entry == NULL) {
+		giterr_set(GITERR_REFERENCE, "No reflog entry at index "PRIuZ, idx);
 		return GIT_ENOTFOUND;
+	}
 
 	reflog_entry_free(entry);
 

src/refs.c

@@ -844,8 +844,10 @@ static int reference__update_terminal(
 	git_reference *ref;
 	int error = 0;
 
-	if (nesting > MAX_NESTING_LEVEL)
+	if (nesting > MAX_NESTING_LEVEL) {
+		giterr_set(GITERR_REFERENCE, "Reference chain too deep (%d)", nesting);
 		return GIT_ENOTFOUND;
+	}
 
 	error = git_reference_lookup(&ref, repo, ref_name);
 

src/remote.c

@@ -63,8 +63,10 @@ static int download_tags_value(git_remote *remote, git_config *cfg)
 	else if (!error && !strcmp(val, "--tags"))
 		remote->download_tags = GIT_REMOTE_DOWNLOAD_TAGS_ALL;
 
-	if (error == GIT_ENOTFOUND)
+	if (error == GIT_ENOTFOUND) {
+		giterr_clear();
 		error = 0;
+	}
 
 	return error;
 }
@@ -210,6 +212,31 @@ static int refspec_cb(const git_config_entry *entry, void *payload)
 	return add_refspec(data->remote, entry->value, data->fetch);
 }
 
+static int get_optional_config(
+	git_config *config, git_buf *buf, git_config_foreach_cb cb, void *payload)
+{
+	int error = 0;
+	const char *key = git_buf_cstr(buf);
+
+	if (git_buf_oom(buf))
+		return -1;
+
+	if (cb != NULL)
+		error = git_config_get_multivar(config, key, NULL, cb, payload);
+	else
+		error = git_config_get_string(payload, config, key);
+
+	if (error == GIT_ENOTFOUND) {
+		giterr_clear();
+		error = 0;
+	}
+
+	if (error < 0)
+		error = -1;
+
+	return error;
+}
+
 int git_remote_load(git_remote **out, git_repository *repo, const char *name)
 {
 	git_remote *remote;
@@ -250,7 +277,7 @@ int git_remote_load(git_remote **out, git_repository *repo, const char *name)
 
 	if ((error = git_config_get_string(&val, config, git_buf_cstr(&buf))) < 0)
 		goto cleanup;
-	
+
 	if (strlen(val) == 0) {
 		giterr_set(GITERR_INVALID, "Malformed remote '%s' - missing URL", name);
 		error = -1;
@@ -261,60 +288,32 @@ int git_remote_load(git_remote **out, git_repository *repo, const char *name)
 	remote->url = git__strdup(val);
 	GITERR_CHECK_ALLOC(remote->url);
 
+	val = NULL;
 	git_buf_clear(&buf);
-	if (git_buf_printf(&buf, "remote.%s.pushurl", name) < 0) {
-		error = -1;
-		goto cleanup;
-	}
-
-	error = git_config_get_string(&val, config, git_buf_cstr(&buf));
-	if (error == GIT_ENOTFOUND) {
-		val = NULL;
-		error = 0;
-	}
+	git_buf_printf(&buf, "remote.%s.pushurl", name);
 
-	if (error < 0) {
-		error = -1;
+	if ((error = get_optional_config(config, &buf, NULL, &val)) < 0)
 		goto cleanup;
-	}
 
 	if (val) {
 		remote->pushurl = git__strdup(val);
 		GITERR_CHECK_ALLOC(remote->pushurl);
 	}
 
-	git_buf_clear(&buf);
-	if (git_buf_printf(&buf, "remote.%s.fetch", name) < 0) {
-		error = -1;
-		goto cleanup;
-	}
-
 	data.remote = remote;
 	data.fetch = true;
-	error = git_config_get_multivar(config, git_buf_cstr(&buf), NULL, refspec_cb, &data);
-	if (error == GIT_ENOTFOUND)
-		error = 0;
-
-	if (error < 0) {
-		error = -1;
-		goto cleanup;
-	}
-
 	git_buf_clear(&buf);
-	if (git_buf_printf(&buf, "remote.%s.push", name) < 0) {
-		error = -1;
+	git_buf_printf(&buf, "remote.%s.fetch", name);
+
+	if ((error = get_optional_config(config, &buf, refspec_cb, &data)) < 0)
 		goto cleanup;
-	}
 
 	data.fetch = false;
-	error = git_config_get_multivar(config, git_buf_cstr(&buf), NULL, refspec_cb, &data);
-	if (error == GIT_ENOTFOUND)
-		error = 0;
+	git_buf_clear(&buf);
+	git_buf_printf(&buf, "remote.%s.push", name);
 
-	if (error < 0) {
-		error = -1;
+	if ((error = get_optional_config(config, &buf, refspec_cb, &data)) < 0)
 		goto cleanup;
-	}
 
 	if (download_tags_value(remote, config) < 0)
 		goto cleanup;
@@ -336,7 +335,7 @@ static int update_config_refspec(const git_remote *remote, git_config *config, i
 	int push;
 	const char *dir;
 	size_t i;
-	int error = -1;
+	int error = 0;
 
 	push = direction == GIT_DIRECTION_PUSH;
 	dir = push ? "push" : "fetch";
@@ -345,9 +344,8 @@ static int update_config_refspec(const git_remote *remote, git_config *config, i
 		return -1;
 
 	/* Clear out the existing config */
-	do {
+	while (!error)
 		error = git_config_delete_entry(config, git_buf_cstr(&name));
-	} while (!error);
 
 	if (error != GIT_ENOTFOUND)
 		return error;
@@ -358,7 +356,8 @@ static int update_config_refspec(const git_remote *remote, git_config *config, i
 		if (spec->push != push)
 			continue;
 
-		if ((error = git_config_set_multivar(config, git_buf_cstr(&name), "", spec->string)) < 0) {
+		if ((error = git_config_set_multivar(
+				config, git_buf_cstr(&name), "", spec->string)) < 0) {
 			goto cleanup;
 		}
 	}

src/repository.c

@@ -1598,6 +1598,7 @@ int git_repository_message(char *buffer, size_t len, git_repository *repo)
 	if ((error = p_stat(git_buf_cstr(&path), &st)) < 0) {
 		if (errno == ENOENT)
 			error = GIT_ENOTFOUND;
+		giterr_set(GITERR_OS, "Could not access message file");
 	}
 	else if (buffer != NULL) {
 		error = git_futils_readbuffer(&buf, git_buf_cstr(&path));

src/revparse.c

@@ -17,7 +17,7 @@
 static int disambiguate_refname(git_reference **out, git_repository *repo, const char *refname)
 {
 	int error = 0, i;
-	bool fallbackmode = true;
+	bool fallbackmode = true, foundvalid = false;
 	git_reference *ref;
 	git_buf refnamebuf = GIT_BUF_INIT, name = GIT_BUF_INIT;
 
@@ -49,6 +49,7 @@ static int disambiguate_refname(git_reference **out, git_repository *repo, const
 			error = GIT_EINVALIDSPEC;
 			continue;
 		}
+		foundvalid = true;
 
 		error = git_reference_lookup_resolved(&ref, repo, git_buf_cstr(&refnamebuf), -1);
 
@@ -63,6 +64,12 @@ static int disambiguate_refname(git_reference **out, git_repository *repo, const
 	}
 
 cleanup:
+	if (error && !foundvalid) {
+		/* never found a valid reference name */
+		giterr_set(GITERR_REFERENCE,
+			"Could not use '%s' as valid reference name", git_buf_cstr(&name));
+	}
+
 	git_buf_free(&name);
 	git_buf_free(&refnamebuf);
 	return error;

src/stash.c

@@ -587,8 +587,10 @@ int git_stash_foreach(
 	const git_reflog_entry *entry;
 
 	error = git_reference_lookup(&stash, repo, GIT_REFS_STASH_FILE);
-	if (error == GIT_ENOTFOUND)
+	if (error == GIT_ENOTFOUND) {
+		giterr_clear();
 		return 0;
+	}
 	if (error < 0)
 		goto cleanup;
 
@@ -651,7 +653,7 @@ int git_stash_drop(
 		const git_reflog_entry *entry;
 
 		entry = git_reflog_entry_byindex(reflog, 0);
-		
+
 		git_reference_free(stash);
 		error = git_reference_create(&stash, repo, GIT_REFS_STASH_FILE, &entry->oid_cur, 1);
 	}

src/status.c

@@ -141,7 +141,7 @@ int git_status_foreach_ext(
 	/* if there is no HEAD, that's okay - we'll make an empty iterator */
 	if (((err = git_repository_head_tree(&head, repo)) < 0) &&
 		!(err == GIT_ENOTFOUND || err == GIT_EORPHANEDHEAD))
-			return err;
+		return err;
 
 	memcpy(&diffopt.pathspec, &opts->pathspec, sizeof(diffopt.pathspec));
 
@@ -238,7 +238,7 @@ static int get_one_status(const char *path, unsigned int status, void *data)
 		 p_fnmatch(sfi->expected, path, sfi->fnm_flags) != 0))
 	{
 		sfi->ambiguous = true;
-		return GIT_EAMBIGUOUS;
+		return GIT_EAMBIGUOUS; /* giterr_set will be done by caller */
 	}
 
 	return 0;

src/submodule.c

@@ -128,6 +128,10 @@ int git_submodule_lookup(
 			git_buf_free(&path);
 		}
 
+		giterr_set(GITERR_SUBMODULE, (error == GIT_ENOTFOUND) ?
+			"No submodule named '%s'" :
+			"Submodule '%s' has not been added yet", name);
+
 		return error;
 	}
 

src/tree.c

@@ -149,7 +149,7 @@ static int tree_key_search(
 	/* Initial homing search; find an entry on the tree with
 	 * the same prefix as the filename we're looking for */
 	if (git_vector_bsearch2(&homing, entries, &homing_search_cmp, &ksearch) < 0)
-		return GIT_ENOTFOUND;
+		return GIT_ENOTFOUND; /* just a signal error; not passed back to user */
 
 	/* We found a common prefix. Look forward as long as
 	 * there are entries that share the common prefix */

tests-clar/object/peel.c

@@ -103,8 +103,3 @@ void test_object_peel__target_any_object_for_type_change(void)
 	/* fail to peel blob */
 	assert_peel_error(GIT_ENOTFOUND, "0266163a49e280c4f5ed1e08facd36a2bd716bcf", GIT_OBJ_ANY);
 }
-
-void test_object_peel__should_use_a_well_known_type(void)
-{
-	assert_peel_error(GIT_EINVALIDSPEC, "7b4384978d2493e851f9cca7858815fac9b10980", GIT_OBJ__EXT2);
-}

tests-clar/refs/branches/remote.c

@@ -49,16 +49,20 @@ void test_refs_branches_remote__no_matching_remote_returns_error(void)
 {
 	const char *unknown = "refs/remotes/nonexistent/master";
 
+	giterr_clear();
 	cl_git_fail_with(git_branch_remote_name(
 		NULL, 0, g_repo, unknown), GIT_ENOTFOUND);
+	cl_assert(giterr_last() != NULL);
 }
 
 void test_refs_branches_remote__local_remote_returns_error(void)
 {
 	const char *local = "refs/heads/master";
 
+	giterr_clear();
 	cl_git_fail_with(git_branch_remote_name(
 		NULL, 0, g_repo, local), GIT_ERROR);
+	cl_assert(giterr_last() != NULL);
 }
 
 void test_refs_branches_remote__ambiguous_remote_returns_error(void)
@@ -75,6 +79,8 @@ void test_refs_branches_remote__ambiguous_remote_returns_error(void)
 
 	git_remote_free(remote);
 
+	giterr_clear();
 	cl_git_fail_with(git_branch_remote_name(NULL, 0, g_repo,
 		remote_tracking_branch_name), GIT_EAMBIGUOUS);
+	cl_assert(giterr_last() != NULL);
 }