error-handling: Repository

This also includes droping `git_buf_lasterror` because it makes no sense
in the new system. Note that in most of the places were it has been
dropped, the code needs cleanup. I.e. GIT_ENOMEM is going away, so
instead it should return a generic `-1` and obviously not throw
anything.

include/git2/errors.h

@@ -125,7 +125,7 @@ typedef enum {
 	GITERR_OS,
 	GITERR_REFERENCE,
 	GITERR_ZLIB,
-
+	GITERR_REPOSITORY,
 } git_error_class;
 
 #define GITERR_CHECK_ALLOC(ptr) if (ptr == NULL) { return -1; }

src/buffer.c

@@ -70,7 +70,7 @@ int git_buf_try_grow(git_buf *buf, size_t target_size)
 
 	new_ptr = git__realloc(new_ptr, new_size);
 	if (!new_ptr)
-		return GIT_ENOMEM;
+		return -1;
 
 	buf->asize = new_size;
 	buf->ptr   = new_ptr;
@@ -100,16 +100,11 @@ void git_buf_clear(git_buf *buf)
 		buf->ptr[0] = '\0';
 }
 
-int git_buf_oom(const git_buf *buf)
+bool git_buf_oom(const git_buf *buf)
 {
 	return (buf->ptr == &git_buf__oom);
 }
 
-int git_buf_lasterror(const git_buf *buf)
-{
-	return (buf->ptr == &git_buf__oom) ? GIT_ENOMEM : GIT_SUCCESS;
-}
-
 int git_buf_set(git_buf *buf, const char *data, size_t len)
 {
 	if (len == 0 || data == NULL) {

src/buffer.h

@@ -57,15 +57,10 @@ void git_buf_attach(git_buf *buf, char *ptr, size_t asize);
  * further calls to modify the buffer will fail.  Check git_buf_oom() at the
  * end of your sequence and it will be true if you ran out of memory at any
  * point with that buffer.
- * @return 0 if no error, 1 if allocation error.
- */
-int git_buf_oom(const git_buf *buf);
-
-/**
- * Just like git_buf_oom, except returns appropriate error code.
- * @return GIT_ENOMEM if allocation error, GIT_SUCCESS if not.
+ *
+ * @return false if no error, true if allocation error
  */
-int git_buf_lasterror(const git_buf *buf);
+bool git_buf_oom(const git_buf *buf);
 
 /*
  * The functions below that return int values, will return GIT_ENOMEM

src/commit.c

@@ -129,7 +129,7 @@ int git_commit_create(
 	git_buf_puts(&commit, message);
 
 	if (git_buf_oom(&commit)) {
-		error = git__throw(git_buf_lasterror(&commit),
+		error = git__throw(GIT_ENOMEM,
 			"Not enough memory to build the commit data");
 		goto cleanup;
 	}

src/crlf.c

@@ -128,8 +128,7 @@ static int drop_crlf(git_buf *dest, const git_buf *source)
 
 	/* Copy remaining input into dest */
 	git_buf_put(dest, scan, scan_end - scan);
-
-	return git_buf_lasterror(dest);
+	return 0;
 }
 
 static int crlf_apply_to_odb(git_filter *self, git_buf *dest, const git_buf *source)

src/diff_output.c

@@ -482,8 +482,8 @@ static int print_compact(void *data, git_diff_delta *delta, float progress)
 	else
 		git_buf_printf(pi->buf, "%c\t%s\n", code, delta->old.path);
 
-	if (git_buf_lasterror(pi->buf) != GIT_SUCCESS)
-		return git_buf_lasterror(pi->buf);
+	if (git_buf_oom(pi->buf) == true)
+		return GIT_ENOMEM;
 
 	return pi->print_cb(pi->cb_data, GIT_DIFF_LINE_FILE_HDR, pi->buf->ptr);
 }
@@ -534,7 +534,10 @@ static int print_oid_range(diff_print_info *pi, git_diff_delta *delta)
 		git_buf_printf(pi->buf, "index %s..%s\n", start_oid, end_oid);
 	}
 
-	return git_buf_lasterror(pi->buf);
+	if (git_buf_oom(pi->buf))
+		return GIT_ENOMEM;
+
+	return 0;
 }
 
 static int print_patch_file(void *data, git_diff_delta *delta, float progress)
@@ -567,8 +570,8 @@ static int print_patch_file(void *data, git_diff_delta *delta, float progress)
 		git_buf_printf(pi->buf, "+++ %s%s\n", newpfx, newpath);
 	}
 
-	if (git_buf_lasterror(pi->buf) != GIT_SUCCESS)
-		return git_buf_lasterror(pi->buf);
+	if (git_buf_oom(pi->buf))
+		return GIT_ENOMEM;
 
 	error = pi->print_cb(pi->cb_data, GIT_DIFF_LINE_FILE_HDR, pi->buf->ptr);
 	if (error != GIT_SUCCESS || delta->binary != 1)
@@ -578,8 +581,8 @@ static int print_patch_file(void *data, git_diff_delta *delta, float progress)
 	git_buf_printf(
 		pi->buf, "Binary files %s%s and %s%s differ\n",
 		oldpfx, oldpath, newpfx, newpath);
-	if (git_buf_lasterror(pi->buf) != GIT_SUCCESS)
-		return git_buf_lasterror(pi->buf);
+	if (git_buf_oom(pi->buf))
+		return GIT_ENOMEM;
 
 	return pi->print_cb(pi->cb_data, GIT_DIFF_LINE_BINARY, pi->buf->ptr);
 }
@@ -601,7 +604,7 @@ static int print_patch_hunk(
 	if (git_buf_printf(pi->buf, "%.*s", (int)header_len, header) == GIT_SUCCESS)
 		return pi->print_cb(pi->cb_data, GIT_DIFF_LINE_HUNK_HDR, pi->buf->ptr);
 	else
-		return git_buf_lasterror(pi->buf);
+		return GIT_ENOMEM;
 }
 
 static int print_patch_line(
@@ -624,8 +627,8 @@ static int print_patch_line(
 	else if (content_len > 0)
 		git_buf_printf(pi->buf, "%.*s", (int)content_len, content);
 
-	if (git_buf_lasterror(pi->buf) != GIT_SUCCESS)
-		return git_buf_lasterror(pi->buf);
+	if (git_buf_oom(pi->buf))
+		return GIT_ENOMEM;
 
 	return pi->print_cb(pi->cb_data, line_origin, pi->buf->ptr);
 }

src/fileops.c

@@ -211,20 +211,21 @@ void git_futils_mmap_free(git_map *out)
 
 int git_futils_mkdir_r(const char *path, const char *base, const mode_t mode)
 {
-	int error, root_path_offset;
+	int root_path_offset;
 	git_buf make_path = GIT_BUF_INIT;
 	size_t start;
 	char *pp, *sp;
+	bool failed = false;
 
 	if (base != NULL) {
 		start = strlen(base);
-		error = git_buf_joinpath(&make_path, base, path);
+		if (git_buf_joinpath(&make_path, base, path) < 0)
+			return -1;
 	} else {
 		start = 0;
-		error = git_buf_puts(&make_path, path);
+		if (git_buf_puts(&make_path, path) < 0)
+			return -1;
 	}
-	if (error < GIT_SUCCESS)
-		return git__rethrow(error, "Failed to create `%s` tree structure", path);
 
 	pp = make_path.ptr + start;
 
@@ -232,14 +233,13 @@ int git_futils_mkdir_r(const char *path, const char *base, const mode_t mode)
 	if (root_path_offset > 0)
 		pp += root_path_offset; /* On Windows, will skip the drive name (eg. C: or D:) */
 
-	while (error == GIT_SUCCESS && (sp = strchr(pp, '/')) != NULL) {
+	while (!failed && (sp = strchr(pp, '/')) != NULL) {
 		if (sp != pp && git_path_isdir(make_path.ptr) == false) {
 			*sp = 0;
-			error = p_mkdir(make_path.ptr, mode);
 
 			/* Do not choke while trying to recreate an existing directory */
-			if (errno == EEXIST)
-				error = GIT_SUCCESS;
+			if (p_mkdir(make_path.ptr, mode) < 0 && errno != EEXIST)
+				failed = true;
 
 			*sp = '/';
 		}
@@ -247,18 +247,20 @@ int git_futils_mkdir_r(const char *path, const char *base, const mode_t mode)
 		pp = sp + 1;
 	}
 
-	if (*pp != '\0' && error == GIT_SUCCESS) {
-		error = p_mkdir(make_path.ptr, mode);
-		if (errno == EEXIST)
-			error = GIT_SUCCESS;
+	if (*pp != '\0' && !failed) {
+		if (p_mkdir(make_path.ptr, mode) < 0 && errno != EEXIST)
+			failed = true;
 	}
 
 	git_buf_free(&make_path);
 
-	if (error < GIT_SUCCESS)
-		return git__throw(error, "Failed to recursively create `%s` tree structure", path);
+	if (failed) {
+		giterr_set(GITERR_OS,
+			"Failed to create directory structure at '%s'", path);
+		return -1;
+	}
 
-	return GIT_SUCCESS;
+	return 0;
 }
 
 static int _rmdir_recurs_foreach(void *opaque, git_buf *path)
@@ -407,8 +409,8 @@ cleanup:
 
 int git_futils_find_system_file(git_buf *path, const char *filename)
 {
-	if (git_buf_joinpath(path, "/etc", filename) < GIT_SUCCESS)
-		return git_buf_lasterror(path);
+	if (git_buf_joinpath(path, "/etc", filename) < 0)
+		return -1;
 
 	if (git_path_exists(path->ptr) == true)
 		return GIT_SUCCESS;

src/indexer.c

@@ -171,7 +171,10 @@ static int index_path(git_buf *path, git_indexer *idx)
 	path->size += GIT_OID_HEXSZ;
 	git_buf_puts(path, suffix);
 
-	return git_buf_lasterror(path);
+	if (git_buf_oom(path))
+		return GIT_ENOMEM;
+
+	return 0;
 }
 
 int git_indexer_write(git_indexer *idx)
@@ -192,8 +195,8 @@ int git_indexer_write(git_indexer *idx)
 	git_buf_truncate(&filename, filename.size - strlen("pack"));
 	git_buf_puts(&filename, "idx");
 
-	if ((error = git_buf_lasterror(&filename)) < GIT_SUCCESS)
-		goto cleanup;
+	if (git_buf_oom(&filename))
+		return GIT_ENOMEM;
 
 	error = git_filebuf_open(&idx->file, filename.ptr, GIT_FILEBUF_HASH_CONTENTS);
 	if (error < GIT_SUCCESS)

src/odb_loose.c

@@ -387,8 +387,8 @@ static int read_loose(git_rawobj *out, git_buf *loc)
 
 	assert(out && loc);
 
-	if ((error = git_buf_lasterror(loc)) < GIT_SUCCESS)
-		return error;
+	if (git_buf_oom(loc))
+		return GIT_ENOMEM;
 
 	out->data = NULL;
 	out->len = 0;
@@ -413,8 +413,8 @@ static int read_header_loose(git_rawobj *out, git_buf *loc)
 
 	assert(out && loc);
 
-	if ((error = git_buf_lasterror(loc)) < GIT_SUCCESS)
-		return error;
+	if (git_buf_oom(loc))
+		return GIT_ENOMEM;
 
 	out->data = NULL;
 
@@ -704,8 +704,8 @@ static int loose_backend__stream_fwrite(git_oid *oid, git_odb_stream *_stream)
 	if ((error = object_file_name(&final_path, backend->objects_dir, oid)) < GIT_SUCCESS)
 		goto cleanup;
 
-	if ((error = git_buf_lasterror(&final_path)) < GIT_SUCCESS)
-		goto cleanup;
+	if (git_buf_oom(&final_path))
+		return GIT_ENOMEM;
 
 	if ((error = git_futils_mkpath2file(final_path.ptr, GIT_OBJECT_DIR_MODE)) < GIT_SUCCESS)
 		goto cleanup;

src/path.c

@@ -56,7 +56,7 @@ Exit:
 
 	if (buffer != NULL) {
 		if (git_buf_set(buffer, startp, len) < GIT_SUCCESS)
-			return git__rethrow(git_buf_lasterror(buffer),
+			return git__rethrow(GIT_ENOMEM,
 				"Could not get basename of '%s'", path);
 	}
 
@@ -116,7 +116,7 @@ Exit:
 
 	if (buffer != NULL) {
 		if (git_buf_set(buffer, path, len) < GIT_SUCCESS)
-			return git__rethrow(git_buf_lasterror(buffer),
+			return git__rethrow(GIT_ENOMEM,
 				"Could not get dirname of '%s'", path);
 	}
 
@@ -185,34 +185,36 @@ int git_path_root(const char *path)
 
 int git_path_prettify(git_buf *path_out, const char *path, const char *base)
 {
-	int  error = GIT_SUCCESS;
 	char buf[GIT_PATH_MAX];
 
+	assert(path && path_out);
 	git_buf_clear(path_out);
 
 	/* construct path if needed */
 	if (base != NULL && git_path_root(path) < 0) {
-		if ((error = git_buf_joinpath(path_out, base, path)) < GIT_SUCCESS)
-			return error;
+		if (git_buf_joinpath(path_out, base, path) < 0)
+			return -1;
+
 		path = path_out->ptr;
 	}
 
-	if (path == NULL || p_realpath(path, buf) == NULL)
-		error = GIT_EOSERR;
-	else
-		error = git_buf_sets(path_out, buf);
+	if (p_realpath(path, buf) == NULL) {
+		giterr_set(GITERR_OS, "Failed to resolve path '%s': %s", path, strerror(errno));
+		return (errno == ENOENT) ? GIT_ENOTFOUND : -1;
+	}
 
-	return error;
+	if (git_buf_sets(path_out, buf) < 0)
+		return -1;
+
+	return 0;
 }
 
 int git_path_prettify_dir(git_buf *path_out, const char *path, const char *base)
 {
-	int error = git_path_prettify(path_out, path, base);
-
-	if (error == GIT_SUCCESS)
-		error = git_path_to_dir(path_out);
+	if (git_path_prettify(path_out, path, base) < 0)
+		return -1;
 
-	return error;
+	return git_path_to_dir(path_out);
 }
 
 int git_path_to_dir(git_buf *path)
@@ -222,7 +224,10 @@ int git_path_to_dir(git_buf *path)
 		path->ptr[path->size - 1] != '/')
 		git_buf_putc(path, '/');
 
-	return git_buf_lasterror(path);
+	if (git_buf_oom(path))
+		return -1;
+
+	return 0;
 }
 
 void git_path_string_to_dir(char* path, size_t size)
@@ -445,7 +450,7 @@ int git_path_find_dir(git_buf *dir, const char *path, const char *base)
 	/* 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_buf_lasterror(dir);
+			error = GIT_ENOMEM;
 
 	if (error == GIT_SUCCESS)
 		error = git_path_to_dir(dir);

src/reflog.c

@@ -65,9 +65,9 @@ static int reflog_write(const char *log_path, const char *oid_old,
 
 	git_buf_putc(&log, '\n');
 
-	if ((error = git_buf_lasterror(&log)) < GIT_SUCCESS) {
+	if (git_buf_oom(&log)) {
 		git_buf_free(&log);
-		return git__rethrow(error, "Failed to write reflog. Memory allocation failure");
+		return git__throw(GIT_ENOMEM, "Failed to write reflog. Memory allocation failure");
 	}
 
 	if ((error = git_filebuf_open(&fbuf, log_path, GIT_FILEBUF_APPEND)) < GIT_SUCCESS) {

src/refspec.c

@@ -97,7 +97,7 @@ int git_refspec_transform(char *out, size_t outlen, const git_refspec *spec, con
 int git_refspec_transform_r(git_buf *out, const git_refspec *spec, const char *name)
 {
 	if (git_buf_sets(out, spec->dst) < GIT_SUCCESS)
-		return git_buf_lasterror(out);
+		return GIT_ENOMEM;
 
 	/*
 	 * No '*' at the end means that it's mapped to one specific local
@@ -109,5 +109,9 @@ int git_refspec_transform_r(git_buf *out, const git_refspec *spec, const char *n
 	git_buf_truncate(out, out->size - 1); /* remove trailing '*' */
 	git_buf_puts(out, name + strlen(spec->src) - 1);
 
-	return git_buf_lasterror(out);
+	if (git_buf_oom(out))
+		return GIT_ENOMEM;
+
+	return 0;
 }
+

src/status.c

@@ -423,10 +423,8 @@ static int dirent_cb(void *state, git_buf *a)
 			if (git_tree_entry_type(m) == GIT_OBJ_TREE)
 				git_path_to_dir(&st->head_tree_relative_path);
 
-			error = git_buf_lasterror(&st->head_tree_relative_path);
-			if (error < GIT_SUCCESS)
-				return git__rethrow(error, "An error occured while "
-					"determining the status of '%s'", a->ptr);
+			if (git_buf_oom(&st->head_tree_relative_path))
+				return GIT_ENOMEM;
 
 			m_name = st->head_tree_relative_path.ptr;
 		} else

src/tag.c

@@ -193,10 +193,9 @@ static int write_tag_annotation(
 	git_buf_putc(&tag, '\n');
 	git_buf_puts(&tag, message);
 
-	error = git_buf_lasterror(&tag);
-	if (error < GIT_SUCCESS) {
+	if (git_buf_oom(&tag)) {
 		git_buf_free(&tag);
-		return git__rethrow(error, "Not enough memory to build the tag data");
+		return git__throw(GIT_ENOMEM, "Not enough memory to build the tag data");
 	}
 
 	error = git_repository_odb__weakptr(&odb, repo);

src/transports/git.c

@@ -68,7 +68,10 @@ static int gen_proto(git_buf *request, const char *cmd, const char *url)
 	git_buf_put(request, url, delim - url);
 	git_buf_putc(request, '\0');
 
-	return git_buf_lasterror(request);
+	if (git_buf_oom(request))
+		return GIT_ENOMEM;
+
+	return 0;
 }
 
 static int send_request(GIT_SOCKET s, const char *cmd, const char *url)

src/transports/http.c

@@ -77,7 +77,10 @@ static int gen_request(git_buf *buf, const char *url, const char *host, const ch
 	}
 	git_buf_puts(buf, "\r\n");
 
-	return git_buf_lasterror(buf);
+	if (git_buf_oom(buf))
+		return GIT_ENOMEM;
+
+	return 0;
 }
 
 static int do_connect(transport_http *t, const char *host, const char *port)

src/tree.c

@@ -569,9 +569,9 @@ int git_treebuilder_write(git_oid *oid, git_repository *repo, git_treebuilder *b
 		git_buf_put(&tree, (char *)entry->oid.id, GIT_OID_RAWSZ);
 	}
 
-	if ((error = git_buf_lasterror(&tree)) < GIT_SUCCESS) {
+	if (git_buf_oom(&tree)) {
 		git_buf_free(&tree);
-		return git__rethrow(error, "Not enough memory to build the tree data");
+		return git__throw(GIT_ENOMEM, "Not enough memory to build the tree data");
 	}
 
 	error = git_repository_odb__weakptr(&odb, repo);
@@ -720,8 +720,9 @@ static int tree_walk_post(
 			/* append the next entry to the path */
 			git_buf_puts(path, entry->filename);
 			git_buf_putc(path, '/');
-			if ((error = git_buf_lasterror(path)) < GIT_SUCCESS)
-				break;
+
+			if (git_buf_oom(path))
+				return GIT_ENOMEM;
 
 			error = tree_walk_post(subtree, callback, path, payload);
 			if (error < GIT_SUCCESS)

src/util.h

@@ -7,6 +7,8 @@
 #ifndef INCLUDE_util_h__
 #define INCLUDE_util_h__
 
+#include "git2/errors.h"
+
 #define ARRAY_SIZE(x) (sizeof(x)/sizeof(x[0]))
 #define bitsizeof(x) (CHAR_BIT * sizeof(x))
 #define MSB(x, bits) ((x) & (~0ULL << (bitsizeof(x) - (bits))))
@@ -23,7 +25,7 @@ GIT_INLINE(void *) git__malloc(size_t len)
 {
 	void *ptr = malloc(len);
 	if (!ptr)
-		git__throw(GIT_ENOMEM, "Out of memory. Failed to allocate %d bytes.", (int)len);
+		giterr_set(GITERR_NOMEMORY, "Out of memory. Failed to allocate %d bytes.", (int)len);
 	return ptr;
 }
 
@@ -31,7 +33,7 @@ GIT_INLINE(void *) git__calloc(size_t nelem, size_t elsize)
 {
 	void *ptr = calloc(nelem, elsize);
 	if (!ptr)
-		git__throw(GIT_ENOMEM, "Out of memory. Failed to allocate %d bytes.", (int)elsize);
+		giterr_set(GITERR_NOMEMORY, "Out of memory. Failed to allocate %d bytes.", (int)nelem*elsize);
 	return ptr;
 }
 
@@ -39,7 +41,7 @@ GIT_INLINE(char *) git__strdup(const char *str)
 {
 	char *ptr = strdup(str);
 	if (!ptr)
-		git__throw(GIT_ENOMEM, "Out of memory. Failed to duplicate string");
+		giterr_set(GITERR_NOMEMORY, "Out of memory. Failed to duplicate string");
 	return ptr;
 }
 
@@ -54,7 +56,7 @@ GIT_INLINE(char *) git__strndup(const char *str, size_t n)
 
 	ptr = (char*)malloc(length + 1);
 	if (!ptr) {
-		git__throw(GIT_ENOMEM, "Out of memory. Failed to duplicate string");
+		giterr_set(GITERR_NOMEMORY, "Out of memory. Failed to duplicate string");
 		return NULL;
 	}
 
@@ -68,7 +70,7 @@ GIT_INLINE(void *) git__realloc(void *ptr, size_t size)
 {
 	void *new_ptr = realloc(ptr, size);
 	if (!new_ptr)
-		git__throw(GIT_ENOMEM, "Out of memory. Failed to allocate %d bytes.", (int)size);
+		giterr_set(GITERR_NOMEMORY, "Out of memory. Failed to allocate %d bytes.", (int)size);
 	return new_ptr;
 }
 

src/win32/posix_w32.c

-/*
+/ < 0)
  * Copyright (C) 2009-2012 the libgit2 contributors
  *
  * This file is part of libgit2, distributed under the GNU GPL v2 with
@@ -288,18 +288,13 @@ int p_rmdir(const char* path)
 
 int p_hide_directory__w32(const char *path)
 {
-	int error;
+	int res;
 	wchar_t* buf = gitwin_to_utf16(path);
 
-	error = SetFileAttributesW(buf, FILE_ATTRIBUTE_HIDDEN) != 0 ?
-		GIT_SUCCESS : GIT_ERROR; /* MSDN states a "non zero" value indicates a success */
-
+	res = SetFileAttributesW(buf, FILE_ATTRIBUTE_HIDDEN);
 	git__free(buf);
-
-	if (error < GIT_SUCCESS)
-		error = git__throw(GIT_EOSERR, "Failed to hide directory '%s'", path);
-
-	return error;
+	
+	return (res != 0) ?  GIT_SUCCESS : GIT_ERROR; /* MSDN states a "non zero" value indicates a success */
 }
 
 char *p_realpath(const char *orig_path, char *buffer)

tests/t12-repo.c

@@ -99,11 +99,15 @@ static int append_ceiling_dir(git_buf *ceiling_dirs, const char *path)
 
 	if (ceiling_dirs->size > 0)
 		git_buf_puts(ceiling_dirs, ceiling_separator);
+
 	git_buf_puts(ceiling_dirs, pretty_path.ptr);
 
 	git_buf_free(&pretty_path);
 
-	return git_buf_lasterror(ceiling_dirs);
+	if (git_buf_oom(ceiling_dirs))
+		return GIT_ENOMEM;
+
+	return 0;
 }
 
 BEGIN_TEST(discover0, "test discover")
@@ -119,7 +123,7 @@ BEGIN_TEST(discover0, "test discover")
 	must_pass(append_ceiling_dir(&ceiling_dirs_buf, TEMP_REPO_FOLDER));
 	ceiling_dirs = git_buf_cstr(&ceiling_dirs_buf);
 
-	must_be_true(git_repository_discover(repository_path, sizeof(repository_path), DISCOVER_FOLDER, 0, ceiling_dirs) == GIT_ENOTAREPO);
+	must_fail(git_repository_discover(repository_path, sizeof(repository_path), DISCOVER_FOLDER, 0, ceiling_dirs));
 
 	must_pass(git_repository_init(&repo, DISCOVER_FOLDER, 1));
 	must_pass(git_repository_discover(repository_path, sizeof(repository_path), DISCOVER_FOLDER, 0, ceiling_dirs));