Convert attr and other files to new errors

This continues to add other files to the new error handling
style.  I think the only real concerns here are that there are
a couple of error return cases that I have converted to asserts,
but I think that it was the correct thing to do given the new
error style.

src/attr.c

@@ -317,13 +317,12 @@ static int collect_attr_files(
 	const char *workdir = git_repository_workdir(repo);
 	attr_walk_up_info info;
 
-	if ((error = git_attr_cache__init(repo)) < GIT_SUCCESS)
-		goto cleanup;
+	if (git_attr_cache__init(repo) < 0 ||
+		git_vector_init(files, 4, NULL) < 0)
+		return -1;
 
-	if ((error = git_vector_init(files, 4, NULL)) < GIT_SUCCESS)
-		goto cleanup;
-
-	if ((error = git_path_find_dir(&dir, path, workdir)) < GIT_SUCCESS)
+	error = git_path_find_dir(&dir, path, workdir);
+	if (error < 0)
 		goto cleanup;
 
 	/* in precendence order highest to lowest:
@@ -334,13 +333,13 @@ static int collect_attr_files(
 	 */
 
 	error = push_attrs(repo, files, repo->path_repository, GIT_ATTR_FILE_INREPO);
-	if (error < GIT_SUCCESS)
+	if (error < 0)
 		goto cleanup;
 
 	info.repo = repo;
 	info.files = files;
 	error = git_path_walk_up(&dir, workdir, push_one_attr, &info);
-	if (error < GIT_SUCCESS)
+	if (error < 0)
 		goto cleanup;
 
 	if ((error = git_repository_config(&cfg, repo)) == GIT_SUCCESS) {
@@ -352,19 +351,17 @@ static int collect_attr_files(
 		git_config_free(cfg);
 	}
 
-	if (error == GIT_SUCCESS) {
+	if (!error) {
 		error = git_futils_find_system_file(&dir, GIT_ATTR_FILE_SYSTEM);
-		if (error == GIT_SUCCESS)
+		if (!error)
 			error = push_attrs(repo, files, NULL, dir.ptr);
 		else if (error == GIT_ENOTFOUND)
-			error = GIT_SUCCESS;
+			error = 0;
 	}
 
  cleanup:
-	if (error < GIT_SUCCESS) {
-		git__rethrow(error, "Could not get attributes for '%s'", path);
+	if (error < 0)
 		git_vector_free(files);
-	}
 	git_buf_free(&dir);
 
 	return error;
@@ -373,32 +370,29 @@ static int collect_attr_files(
 
 int git_attr_cache__init(git_repository *repo)
 {
-	int error = GIT_SUCCESS;
 	git_attr_cache *cache = &repo->attrcache;
 
 	if (cache->initialized)
-		return GIT_SUCCESS;
+		return 0;
 
 	if (cache->files == NULL) {
 		cache->files = git_hashtable_alloc(
 			8, git_hash__strhash_cb, git_hash__strcmp_cb);
 		if (!cache->files)
-			return git__throw(GIT_ENOMEM, "Could not initialize attribute cache");
+			return -1;
 	}
 
 	if (cache->macros == NULL) {
 		cache->macros = git_hashtable_alloc(
 			8, git_hash__strhash_cb, git_hash__strcmp_cb);
 		if (!cache->macros)
-			return git__throw(GIT_ENOMEM, "Could not initialize attribute cache");
+			return -1;
 	}
 
 	cache->initialized = 1;
 
 	/* insert default macros */
-	error = git_attr_add_macro(repo, "binary", "-diff -crlf");
-
-	return error;
+	return git_attr_add_macro(repo, "binary", "-diff -crlf");
 }
 
 void git_attr_cache_flush(
@@ -432,8 +426,9 @@ void git_attr_cache_flush(
 
 int git_attr_cache__insert_macro(git_repository *repo, git_attr_rule *macro)
 {
+	/* TODO: generate warning log if (macro->assigns.length == 0) */
 	if (macro->assigns.length == 0)
-		return git__throw(GIT_EMISSINGOBJDATA, "git attribute macro with no values");
+		return 0;
 
 	return git_hashtable_insert(
 		repo->attrcache.macros, macro->match.pattern, macro);

src/attr_file.c

@@ -180,37 +180,37 @@ int git_attr_file__lookup_one(
 		}
 	}
 
-	return GIT_SUCCESS;
+	return 0;
 }
 
 
-int git_attr_fnmatch__match(
+bool git_attr_fnmatch__match(
 	git_attr_fnmatch *match,
 	const git_attr_path *path)
 {
-	int matched = FNM_NOMATCH;
+	int fnm;
 
 	if (match->flags & GIT_ATTR_FNMATCH_DIRECTORY && !path->is_dir)
-		return matched;
+		return false;
 
 	if (match->flags & GIT_ATTR_FNMATCH_FULLPATH)
-		matched = p_fnmatch(match->pattern, path->path, FNM_PATHNAME);
+		fnm = p_fnmatch(match->pattern, path->path, FNM_PATHNAME);
 	else if (path->is_dir)
-		matched = p_fnmatch(match->pattern, path->basename, FNM_LEADING_DIR);
+		fnm = p_fnmatch(match->pattern, path->basename, FNM_LEADING_DIR);
 	else
-		matched = p_fnmatch(match->pattern, path->basename, 0);
+		fnm = p_fnmatch(match->pattern, path->basename, 0);
 
-	return matched;
+	return (fnm == FNM_NOMATCH) ? false : true;
 }
 
-int git_attr_rule__match(
+bool git_attr_rule__match(
 	git_attr_rule *rule,
 	const git_attr_path *path)
 {
-	int matched = git_attr_fnmatch__match(&rule->match, path);
+	bool matched = git_attr_fnmatch__match(&rule->match, path);
 
 	if (rule->match.flags & GIT_ATTR_FNMATCH_NEGATIVE)
-		matched = (matched == GIT_SUCCESS) ? FNM_NOMATCH : GIT_SUCCESS;
+		matched = !matched;
 
 	return matched;
 }

src/attr_file.h

@@ -82,7 +82,7 @@ extern int git_attr_file__lookup_one(
 /* loop over rules in file from bottom to top */
 #define git_attr_file__foreach_matching_rule(file, path, iter, rule)	\
 	git_vector_rforeach(&(file)->rules, (iter), (rule)) \
-		if (git_attr_rule__match((rule), (path)) == GIT_SUCCESS)
+		if (git_attr_rule__match((rule), (path)))
 
 extern unsigned long git_attr_file__name_hash(const char *name);
 
@@ -96,13 +96,13 @@ extern int git_attr_fnmatch__parse(
 	const char *source,
 	const char **base);
 
-extern int git_attr_fnmatch__match(
+extern bool git_attr_fnmatch__match(
 	git_attr_fnmatch *rule,
 	const git_attr_path *path);
 
 extern void git_attr_rule__free(git_attr_rule *rule);
 
-extern int git_attr_rule__match(
+extern bool git_attr_rule__match(
 	git_attr_rule *rule,
 	const git_attr_path *path);
 

src/hashtable.c

@@ -31,9 +31,7 @@ static int resize_to(git_hashtable *self, size_t new_size)
 		self->size_mask = new_size - 1;
 		self->key_count = 0;
 		self->nodes = git__calloc(1, sizeof(git_hashtable_node) * self->size);
-
-		if (self->nodes == NULL)
-			return GIT_ENOMEM;
+		GITERR_CHECK_ALLOC(self->nodes);
 
 		if (insert_nodes(self, old_nodes, old_size) == 0)
 			self->is_resizing = 0;
@@ -44,22 +42,22 @@ static int resize_to(git_hashtable *self, size_t new_size)
 	} while(self->is_resizing);
 
 	git__free(old_nodes);
-	return GIT_SUCCESS;
+	return 0;
 }
 
 static int set_size(git_hashtable *self, size_t new_size)
 {
 	self->nodes = git__realloc(self->nodes, new_size * sizeof(git_hashtable_node));
-	if (self->nodes == NULL)
-		return GIT_ENOMEM;
+	GITERR_CHECK_ALLOC(self->nodes);
 
 	if (new_size > self->size) {
-		memset(&self->nodes[self->size], 0x0, (new_size - self->size) * sizeof(git_hashtable_node));
+		memset(&self->nodes[self->size], 0x0,
+			(new_size - self->size) * sizeof(git_hashtable_node));
 	}
 
 	self->size = new_size;
 	self->size_mask = new_size - 1;
-	return GIT_SUCCESS;
+	return 0;
 }
 
 static git_hashtable_node *node_with_hash(git_hashtable *self, const void *key, int hash_id)
@@ -84,43 +82,47 @@ static int node_insert(git_hashtable *self, git_hashtable_node *new_node)
 			git_hashtable_node *node;
 			node = node_with_hash(self, new_node->key, hash_id);
 			node_swap_with(new_node, node);
-			if(new_node->key == 0x0){
+			if (new_node->key == 0x0){
 				self->key_count++;
-				return GIT_SUCCESS;
+				return 0;
 			}
 		}
 	}
 
-	if (self->is_resizing)
-		return git__throw(GIT_EBUSY, "Failed to insert node. Hashtable is currently resizing");
+	/* Failed to insert node. Hashtable is currently resizing */
+	assert(!self->is_resizing);
 
-	resize_to(self, self->size * 2);
-	git_hashtable_insert(self, new_node->key, new_node->value);
-	return GIT_SUCCESS;
+	if (resize_to(self, self->size * 2) < 0)
+		return -1;
+
+	return git_hashtable_insert(self, new_node->key, new_node->value);
 }
 
-static int insert_nodes(git_hashtable *self, git_hashtable_node *old_nodes, size_t old_size)
+static int insert_nodes(
+	git_hashtable *self, git_hashtable_node *old_nodes, size_t old_size)
 {
 	size_t i;
 
 	for (i = 0; i < old_size; ++i) {
 		git_hashtable_node *node = git_hashtable_node_at(old_nodes, i);
-		if (node->key && git_hashtable_insert(self, node->key, node->value) < GIT_SUCCESS)
-			return GIT_ENOMEM;
+		if (node->key &&
+			git_hashtable_insert(self, node->key, node->value) < 0)
+			return -1;
 	}
 
-	return GIT_SUCCESS;
+	return 0;
 }
 
-git_hashtable *git_hashtable_alloc(size_t min_size,
-		git_hash_ptr hash,
-		git_hash_keyeq_ptr key_eq)
+git_hashtable *git_hashtable_alloc(
+	size_t min_size,
+	git_hash_ptr hash,
+	git_hash_keyeq_ptr key_eq)
 {
 	git_hashtable *table;
 
 	assert(hash && key_eq);
 
-	if ((table = git__malloc(sizeof(git_hashtable))) == NULL)
+	if ((table = git__malloc(sizeof(*table))) == NULL)
 		return NULL;
 
 	memset(table, 0x0, sizeof(git_hashtable));
@@ -161,7 +163,8 @@ void git_hashtable_free(git_hashtable *self)
 }
 
 
-int git_hashtable_insert2(git_hashtable *self, const void *key, void *value, void **old_value)
+int git_hashtable_insert2(
+	git_hashtable *self, const void *key, void *value, void **old_value)
 {
 	int hash_id;
 	git_hashtable_node *node;
@@ -177,14 +180,14 @@ int git_hashtable_insert2(git_hashtable *self, const void *key, void *value, voi
 			node->key = key;
 			node->value = value;
 			self->key_count++;
-			return GIT_SUCCESS;
+			return 0;
 		}
 
 		if (key == node->key || self->key_equal(key, node->key) == 0) {
 			*old_value = node->value;
 			node->key = key;
 			node->value = value;
-			return GIT_SUCCESS;
+			return 0;
 		}
 	}
 
@@ -213,7 +216,8 @@ void *git_hashtable_lookup(git_hashtable *self, const void *key)
 	return NULL;
 }
 
-int git_hashtable_remove2(git_hashtable *self, const void *key, void **old_value)
+int git_hashtable_remove2(
+	git_hashtable *self, const void *key, void **old_value)
 {
 	int hash_id;
 	git_hashtable_node *node;
@@ -236,8 +240,8 @@ int git_hashtable_remove2(git_hashtable *self, const void *key, void **old_value
 
 int git_hashtable_merge(git_hashtable *self, git_hashtable *other)
 {
-	if (resize_to(self, (self->size + other->size) * 2) < GIT_SUCCESS)
-		return GIT_ENOMEM;
+	if (resize_to(self, (self->size + other->size) * 2) < 0)
+		return -1;
 
 	return insert_nodes(self, other->nodes, other->key_count);
 }

src/ignore.c

@@ -168,9 +168,9 @@ static int ignore_lookup_in_rules(
 	git_attr_fnmatch *match;
 
 	git_vector_rforeach(rules, j, match) {
-		if (git_attr_fnmatch__match(match, path) == GIT_SUCCESS) {
+		if (git_attr_fnmatch__match(match, path)) {
 			*ignored = ((match->flags & GIT_ATTR_FNMATCH_NEGATIVE) == 0);
-			return GIT_SUCCESS;
+			return 0;
 		}
 	}
 

src/path.c

@@ -54,11 +54,8 @@ int git_path_basename_r(git_buf *buffer, const char *path)
 Exit:
 	result = len;
 
-	if (buffer != NULL) {
-		if (git_buf_set(buffer, startp, len) < GIT_SUCCESS)
-			return git__rethrow(GIT_ENOMEM,
-				"Could not get basename of '%s'", path);
-	}
+	if (buffer != NULL && git_buf_set(buffer, startp, len) < 0)
+		return -1;
 
 	return result;
 }
@@ -114,11 +111,8 @@ int git_path_dirname_r(git_buf *buffer, const char *path)
 Exit:
 	result = len;
 
-	if (buffer != NULL) {
-		if (git_buf_set(buffer, path, len) < GIT_SUCCESS)
-			return git__rethrow(GIT_ENOMEM,
-				"Could not get dirname of '%s'", path);
-	}
+	if (buffer != NULL && git_buf_set(buffer, path, len) < 0)
+		return -1;
 
 	return result;
 }
@@ -199,7 +193,8 @@ int git_path_prettify(git_buf *path_out, const char *path, const char *base)
 	}
 
 	if (p_realpath(path, buf) == NULL) {
-		giterr_set(GITERR_OS, "Failed to resolve path '%s': %s", path, strerror(errno));
+		giterr_set(GITERR_OS, "Failed to resolve path '%s': %s",
+			path, strerror(errno));
 		return (errno == ENOENT) ? GIT_ENOTFOUND : -1;
 	}
 
@@ -211,10 +206,8 @@ int git_path_prettify(git_buf *path_out, const char *path, const char *base)
 
 int git_path_prettify_dir(git_buf *path_out, const char *path, const char *base)
 {
-	if (git_path_prettify(path_out, path, base) < 0)
-		return -1;
-
-	return git_path_to_dir(path_out);
+	int error = git_path_prettify(path_out, path, base);
+	return (error < 0) ? error : git_path_to_dir(path_out);
 }
 
 int git_path_to_dir(git_buf *path)
@@ -224,10 +217,7 @@ int git_path_to_dir(git_buf *path)
 		path->ptr[path->size - 1] != '/')
 		git_buf_putc(path, '/');
 
-	if (git_buf_oom(path))
-		return -1;
-
-	return 0;
+	return git_buf_oom(path) ? -1 : 0;
 }
 
 void git_path_string_to_dir(char* path, size_t size)
@@ -242,7 +232,7 @@ void git_path_string_to_dir(char* path, size_t size)
 
 int git__percent_decode(git_buf *decoded_out, const char *input)
 {
-	int len, hi, lo, i, error = GIT_SUCCESS;
+	int len, hi, lo, i;
 	assert(decoded_out && input);
 
 	len = strlen(input);
@@ -268,24 +258,27 @@ int git__percent_decode(git_buf *decoded_out, const char *input)
 		i += 2;
 
 append:
-		error = git_buf_putc(decoded_out, c);
-		if (error < GIT_SUCCESS)
-			return git__rethrow(error, "Failed to percent decode '%s'.", input);
+		if (git_buf_putc(decoded_out, c) < 0)
+			return -1;
 	}
 
-	return error;
+	return 0;
+}
+
+static int error_invalid_local_file_uri(const char *uri)
+{
+	giterr_set(GITERR_CONFIG, "'%s' is not a valid local file URI", uri);
+	return -1;
 }
 
 int git_path_fromurl(git_buf *local_path_out, const char *file_url)
 {
-	int error = GIT_SUCCESS, offset = 0, len;
+	int offset = 0, len;
 
 	assert(local_path_out && file_url);
 
 	if (git__prefixcmp(file_url, "file://") != 0)
-		return git__throw(GIT_EINVALIDPATH,
-			"Parsing of '%s' failed. A file Uri is expected (ie. with 'file://' scheme).",
-			file_url);
+		return error_invalid_local_file_uri(file_url);
 
 	offset += 7;
 	len = strlen(file_url);
@@ -295,12 +288,10 @@ int git_path_fromurl(git_buf *local_path_out, const char *file_url)
 	else if (offset < len && git__prefixcmp(file_url + offset, "localhost/") == 0)
 		offset += 10;
 	else
-		return git__throw(GIT_EINVALIDPATH,
-			"Parsing of '%s' failed. A local file Uri is expected.", file_url);
+		return error_invalid_local_file_uri(file_url);
 
 	if (offset >= len || file_url[offset] == '/')
-		return git__throw(GIT_EINVALIDPATH, 
-			"Parsing of '%s' failed. Invalid file Uri format.", file_url);
+		return error_invalid_local_file_uri(file_url);
 
 #ifndef _MSC_VER
 	offset--;	/* A *nix absolute path starts with a forward slash */
@@ -308,11 +299,7 @@ int git_path_fromurl(git_buf *local_path_out, const char *file_url)
 
 	git_buf_clear(local_path_out);
 
-	error = git__percent_decode(local_path_out, file_url + offset);
-	if (error < GIT_SUCCESS)
-		return git__rethrow(error, "Parsing of '%s' failed.", file_url);
-
-	return error;
+	return git__percent_decode(local_path_out, file_url + offset);
 }
 
 int git_path_walk_up(
@@ -321,7 +308,7 @@ int git_path_walk_up(
 	int (*cb)(void *data, git_buf *),
 	void *data)
 {
-	int error = GIT_SUCCESS;
+	int error = 0;
 	git_buf iter;
 	ssize_t stop = 0, scan;
 	char oldc = '\0';
@@ -341,7 +328,7 @@ int git_path_walk_up(
 	iter.asize = path->asize;
 
 	while (scan >= stop) {
-		if ((error = cb(data, &iter)) < GIT_SUCCESS)
+		if ((error = cb(data, &iter)) < 0)
 			break;
 		iter.ptr[scan] = oldc;
 		scan = git_buf_rfind_next(&iter, '/');
@@ -434,25 +421,24 @@ bool git_path_contains_file(git_buf *base, const char *file)
 
 int git_path_find_dir(git_buf *dir, const char *path, const char *base)
 {
-	int error = GIT_SUCCESS;
+	int error;
 
 	if (base != NULL && git_path_root(path) < 0)
 		error = git_buf_joinpath(dir, base, path);
 	else
 		error = git_buf_sets(dir, path);
 
-	if (error == GIT_SUCCESS) {
+	if (!error) {
 		char buf[GIT_PATH_MAX];
 		if (p_realpath(dir->ptr, buf) != NULL)
 			error = git_buf_sets(dir, buf);
 	}
 
 	/* call dirname if this is not a directory */
-	if (error == GIT_SUCCESS && git_path_isdir(dir->ptr) == false)
-		if (git_path_dirname_r(dir, dir->ptr) < GIT_SUCCESS)
-			error = GIT_ENOMEM;
+	if (!error && git_path_isdir(dir->ptr) == false)
+		error = git_path_dirname_r(dir, dir->ptr);
 
-	if (error == GIT_SUCCESS)
+	if (!error)
 		error = git_path_to_dir(dir);
 
 	return error;
@@ -497,9 +483,9 @@ int git_path_direach(
 		return -1;
 
 	wd_len = path->size;
-	dir = opendir(path->ptr);
-	if (!dir) {
-		giterr_set(GITERR_OS, "Failed to 'opendir' %s", path->ptr);
+
+	if ((dir = opendir(path->ptr)) == NULL) {
+		giterr_set(GITERR_OS, "Failed to open directory '%s'", path->ptr);
 		return -1;
 	}
 
@@ -541,9 +527,10 @@ int git_path_dirload(
 	path_len = strlen(path);
 	assert(path_len > 0 && path_len >= prefix_len);
 
-	if ((dir = opendir(path)) == NULL)
-		return git__throw(GIT_EOSERR, "Failed to process `%s` tree structure."
-			" An error occured while opening the directory", path);
+	if ((dir = opendir(path)) == NULL) {
+		giterr_set(GITERR_OS, "Failed to open directory '%s'", path);
+		return -1;
+	}
 
 	path += prefix_len;
 	path_len -= prefix_len;
@@ -560,8 +547,7 @@ int git_path_dirload(
 
 		entry_path = git__malloc(
 			path_len + need_slash + entry_len + 1 + alloc_extra);
-		if (entry_path == NULL)
-			return GIT_ENOMEM;
+		GITERR_CHECK_ALLOC(entry_path);
 
 		if (path_len)
 			memcpy(entry_path, path, path_len);
@@ -570,19 +556,16 @@ int git_path_dirload(
 		memcpy(&entry_path[path_len + need_slash], de->d_name, entry_len);
 		entry_path[path_len + need_slash + entry_len] = '\0';
 
-		if ((error = git_vector_insert(contents, entry_path)) < GIT_SUCCESS) {
-			git__free(entry_path);
-			return error;
-		}
+		if (git_vector_insert(contents, entry_path) < 0)
+			return -1;
 	}
 
 	closedir(dir);
 
-	if (error != GIT_SUCCESS)
-		return git__throw(
-			GIT_EOSERR, "Failed to process directory entry in `%s`", path);
+	if (error != 0)
+		giterr_set(GITERR_OS, "Failed to process directory entry in '%s'", path);
 
-	return GIT_SUCCESS;
+	return error;
 }
 
 int git_path_with_stat_cmp(const void *a, const void *b)
@@ -601,11 +584,12 @@ int git_path_dirload_with_stat(
 	git_path_with_stat *ps;
 	git_buf full = GIT_BUF_INIT;
 
-	if ((error = git_buf_set(&full, path, prefix_len)) != GIT_SUCCESS)
-		return error;
+	if (git_buf_set(&full, path, prefix_len) < 0)
+		return -1;
 
-	if ((error = git_path_dirload(path, prefix_len,
-		sizeof(git_path_with_stat) + 1, contents)) != GIT_SUCCESS) {
+	error = git_path_dirload(
+		path, prefix_len, sizeof(git_path_with_stat) + 1, contents);
+	if (error < 0) {
 		git_buf_free(&full);
 		return error;
 	}
@@ -616,8 +600,17 @@ int git_path_dirload_with_stat(
 		memmove(ps->path, ps, path_len + 1);
 		ps->path_len = path_len;
 
-		git_buf_joinpath(&full, full.ptr, ps->path);
-		p_lstat(full.ptr, &ps->st);
+		if (git_buf_joinpath(&full, full.ptr, ps->path) < 0) {
+			error = -1;
+			break;
+		}
+
+		if (p_lstat(full.ptr, &ps->st) < 0) {
+			giterr_set(GITERR_OS, "Failed to stat file '%s'", full.ptr);
+			error = -1;
+			break;
+		}
+
 		git_buf_truncate(&full, prefix_len);
 
 		if (S_ISDIR(ps->st.st_mode)) {