error-handling: References

Yes, this is error handling solely for `refs.c`, but some of the
abstractions leak all ofer the code base.

include/git2/errors.h

@@ -122,16 +122,17 @@ typedef struct {
 
 typedef enum {
 	GITERR_NOMEMORY,
+	GITERR_OS,
 	GITERR_REFERENCE,
+	GITERR_ZLIB,
 
 } git_error_class;
 
-#define GITERR_CHECK_ALLOC(ptr, error) if (ptr == NULL) { giterr_set_oom(error); return -1 }
+#define GITERR_CHECK_ALLOC(ptr) if (ptr == NULL) { return -1; }
 
-GIT_EXTERN(void) giterr_set(git_error **error_out, int error_class, const char *string, ...);
-GIT_EXTERN(void) giterr_set_oom(git_error **error);
-GIT_EXTERN(void) giterr_free(git_error *error);
-GIT_EXTERN(void) giterr_clear(git_error **error);
+GIT_EXTERN(void) giterr_set_oom(void);
+GIT_EXTERN(void) giterr_set(int error_class, const char *string, ...);
+GIT_EXTERN(void) giterr_clear(void);
 
 /**
  * Return a detailed error string with the latest error

src/attr_file.c

@@ -250,11 +250,12 @@ int git_attr_path__init(
 		git_buf full_path = GIT_BUF_INIT;
 		int error = git_buf_joinpath(&full_path, base, path);
 		if (error == GIT_SUCCESS)
-			info->is_dir = (git_path_isdir(full_path.ptr) == GIT_SUCCESS);
+			info->is_dir = (int)git_path_isdir(full_path.ptr);
+
 		git_buf_free(&full_path);
 		return error;
 	}
-	info->is_dir = (git_path_isdir(path) == GIT_SUCCESS);
+	info->is_dir = (int)git_path_isdir(path);
 
 	return GIT_SUCCESS;
 }

src/cc-compat.h

@@ -50,4 +50,12 @@
 #	pragma warning ( disable : 4127 )
 #endif
 
+#if defined (_MSC_VER)
+	typedef unsigned char bool;
+#	define true 1
+#	define false 0
+#else
+#	include <stdbool.h>
+#endif
+
 #endif /* INCLUDE_compat_h__ */

src/errors.c

@@ -108,20 +108,24 @@ void git_clearerror(void)
  * New error handling
  ********************************************/
 
-void giterr_set(git_error **error_out, int error_class, const char *string, ...)
+static git_error g_git_oom_error = {
+	"Out of memory",
+	GITERR_NOMEMORY
+};
+
+void giterr_set_oom(void)
+{
+	GIT_GLOBAL->last_error = &g_git_oom_error;
+}
+
+void giterr_set(int error_class, const char *string, ...)
 {
 	char error_str[1024];
 	va_list arglist;
 	git_error *error;
 
-	if (error_out == NULL)
-		return;
-
-	error = git__malloc(sizeof(git_error));
-	if (!error) {
-		giterr_set_oom(error_out);
-		return;
-	}
+	error = &GIT_GLOBAL->error_t;
+	free(error->message);
 
 	va_start(arglist, string);
 	p_vsnprintf(error_str, sizeof(error_str), string, arglist);
@@ -131,38 +135,14 @@ void giterr_set(git_error **error_out, int error_class, const char *string, ...)
 	error->klass = error_class;
 
 	if (error->message == NULL) {
-		free(error);
-		giterr_set_oom(error_out);
+		giterr_set_oom();
 		return;
 	}
 
-	*error_out = error;
-}
-
-static git_error g_git_oom_error = {
-	"Out of memory",
-	GITERR_NOMEMORY
-};
-
-void giterr_set_oom(git_error **error)
-{
-	if (error != NULL)
-		*error = &g_git_oom_error;
-}
-
-void giterr_free(git_error *error)
-{
-	if (error == NULL || error == &g_git_oom_error)
-		return;
-
-	free(error->message);
-	free(error);
+	GIT_GLOBAL->last_error = error;
 }
 
-void giterr_clear(git_error **error)
+void giterr_clear(void)
 {
-	if (error != NULL) {
-		giterr_free(*error);
-		*error = NULL;
-	}
+	GIT_GLOBAL->last_error = NULL;
 }

src/filebuf.c

@@ -16,11 +16,14 @@ static const size_t WRITE_BUFFER_SIZE = (4096 * 2);
 
 static int lock_file(git_filebuf *file, int flags)
 {
-	if (git_path_exists(file->path_lock) == 0) {
+	if (git_path_exists(file->path_lock) == true) {
 		if (flags & GIT_FILEBUF_FORCE)
 			p_unlink(file->path_lock);
-		else
-			return git__throw(GIT_EOSERR, "Failed to lock file");
+		else {
+			giterr_set(GITERR_OS,
+				"Failed to lock file '%s' for writing", file->path_lock);
+			return -1;
+		}
 	}
 
 	/* create path to the file buffer is required */
@@ -32,16 +35,20 @@ static int lock_file(git_filebuf *file, int flags)
 	}
 
 	if (file->fd < 0)
-		return git__throw(GIT_EOSERR, "Failed to create lock");
+		return -1;
 
-	if ((flags & GIT_FILEBUF_APPEND) && git_path_exists(file->path_original) == 0) {
+	if ((flags & GIT_FILEBUF_APPEND) && git_path_exists(file->path_original) == true) {
 		git_file source;
 		char buffer[2048];
 		size_t read_bytes;
 
 		source = p_open(file->path_original, O_RDONLY);
-		if (source < 0)
-			return git__throw(GIT_EOSERR, "Failed to lock file. Could not open %s", file->path_original);
+		if (source < 0) {
+			giterr_set(GITERR_OS,
+				"Failed to open file '%s' for reading: %s",
+				file->path_original, strerror(errno));
+			return -1;
+		}
 
 		while ((read_bytes = p_read(source, buffer, 2048)) > 0) {
 			p_write(file->fd, buffer, read_bytes);
@@ -60,7 +67,7 @@ void git_filebuf_cleanup(git_filebuf *file)
 	if (file->fd >= 0)
 		p_close(file->fd);
 
-	if (file->fd >= 0 && file->path_lock && git_path_exists(file->path_lock) == GIT_SUCCESS)
+	if (file->fd >= 0 && file->path_lock && git_path_exists(file->path_lock) == true)
 		p_unlink(file->path_lock);
 
 	if (file->digest)
@@ -141,13 +148,13 @@ static int write_deflate(git_filebuf *file, void *source, size_t len)
 
 int git_filebuf_open(git_filebuf *file, const char *path, int flags)
 {
-	int error, compression;
+	int compression;
 	size_t path_len;
 
-	assert(file && path);
-
-	if (file->buffer)
-		return git__throw(GIT_EINVALIDARGS, "Tried to reopen an open filebuf");
+	/* opening an already open buffer is a programming error;
+	 * assert that this never happens instead of returning
+	 * an error code */
+	assert(file && path && file->buffer == NULL);
 
 	memset(file, 0x0, sizeof(git_filebuf));
 
@@ -157,17 +164,12 @@ int git_filebuf_open(git_filebuf *file, const char *path, int flags)
 
 	/* Allocate the main cache buffer */
 	file->buffer = git__malloc(file->buf_size);
-	if (file->buffer == NULL){
-		error = GIT_ENOMEM;
-		goto cleanup;
-	}
+	GITERR_CHECK_ALLOC(file->buffer);
 
 	/* If we are hashing on-write, allocate a new hash context */
 	if (flags & GIT_FILEBUF_HASH_CONTENTS) {
-		if ((file->digest = git_hash_new_ctx()) == NULL) {
-			error = GIT_ENOMEM;
-			goto cleanup;
-		}
+		file->digest = git_hash_new_ctx();
+		GITERR_CHECK_ALLOC(file->digest);
 	}
 
 	compression = flags >> GIT_FILEBUF_DEFLATE_SHIFT;
@@ -176,16 +178,13 @@ int git_filebuf_open(git_filebuf *file, const char *path, int flags)
 	if (compression != 0) {
 		/* Initialize the ZLib stream */
 		if (deflateInit(&file->zs, compression) != Z_OK) {
-			error = git__throw(GIT_EZLIB, "Failed to initialize zlib");
+			giterr_set(GITERR_ZLIB, "Failed to initialize zlib");
 			goto cleanup;
 		}
 
 		/* Allocate the Zlib cache buffer */
 		file->z_buf = git__malloc(file->buf_size);
-		if (file->z_buf == NULL){
-			error = GIT_ENOMEM;
-			goto cleanup;
-		}
+		GITERR_CHECK_ALLOC(file->z_buf);
 
 		/* Never flush */
 		file->flush_mode = Z_NO_FLUSH;
@@ -200,50 +199,40 @@ int git_filebuf_open(git_filebuf *file, const char *path, int flags)
 
 		/* Open the file as temporary for locking */
 		file->fd = git_futils_mktmp(&tmp_path, path);
+
 		if (file->fd < 0) {
 			git_buf_free(&tmp_path);
-			error = GIT_EOSERR;
 			goto cleanup;
 		}
 
 		/* No original path */
 		file->path_original = NULL;
 		file->path_lock = git_buf_detach(&tmp_path);
-
-		if (file->path_lock == NULL) {
-			error = GIT_ENOMEM;
-			goto cleanup;
-		}
+		GITERR_CHECK_ALLOC(file->path_lock);
 	} else {
 		path_len = strlen(path);
 
 		/* Save the original path of the file */
 		file->path_original = git__strdup(path);
-		if (file->path_original == NULL) {
-			error = GIT_ENOMEM;
-			goto cleanup;
-		}
+		GITERR_CHECK_ALLOC(file->path_original);
 
 		/* create the locking path by appending ".lock" to the original */
 		file->path_lock = git__malloc(path_len + GIT_FILELOCK_EXTLENGTH);
-		if (file->path_lock == NULL) {
-			error = GIT_ENOMEM;
-			goto cleanup;
-		}
+		GITERR_CHECK_ALLOC(file->path_lock);
 
 		memcpy(file->path_lock, file->path_original, path_len);
 		memcpy(file->path_lock + path_len, GIT_FILELOCK_EXTENSION, GIT_FILELOCK_EXTLENGTH);
 
 		/* open the file for locking */
-		if ((error = lock_file(file, flags)) < GIT_SUCCESS)
+		if (lock_file(file, flags) < 0)
 			goto cleanup;
 	}
 
-	return GIT_SUCCESS;
+	return 0;
 
 cleanup:
 	git_filebuf_cleanup(file);
-	return git__rethrow(error, "Failed to open file buffer for '%s'", path);
+	return -1;
 }
 
 int git_filebuf_hash(git_oid *oid, git_filebuf *file)

src/fileops.c

@@ -10,25 +10,19 @@
 
 int git_futils_mkpath2file(const char *file_path, const mode_t mode)
 {
-	int error;
+	int result = 0;
 	git_buf target_folder = GIT_BUF_INIT;
 
-	error = git_path_dirname_r(&target_folder, file_path);
-	if (error < GIT_SUCCESS) {
-		git_buf_free(&target_folder);
-		return git__throw(GIT_EINVALIDPATH, "Failed to recursively build `%s` tree structure. Unable to parse parent folder name", file_path);
-	} else {
-		/* reset error */
-		error = GIT_SUCCESS;
-	}
+	if (git_path_dirname_r(&target_folder, file_path) < 0)
+		return -1;
 
 	/* Does the containing folder exist? */
-	if (git_path_isdir(target_folder.ptr) != GIT_SUCCESS)
+	if (git_path_isdir(target_folder.ptr) == false)
 		/* Let's create the tree structure */
-		error = git_futils_mkdir_r(target_folder.ptr, NULL, mode);
+		result = git_futils_mkdir_r(target_folder.ptr, NULL, mode);
 
 	git_buf_free(&target_folder);
-	return error;
+	return result;
 }
 
 int git_futils_mktmp(git_buf *path_out, const char *filename)
@@ -39,33 +33,50 @@ int git_futils_mktmp(git_buf *path_out, const char *filename)
 	git_buf_puts(path_out, "_git2_XXXXXX");
 
 	if (git_buf_oom(path_out))
-		return git__rethrow(git_buf_lasterror(path_out),
-			"Failed to create temporary file for %s", filename);
+		return -1;
 
-	if ((fd = p_mkstemp(path_out->ptr)) < 0)
-		return git__throw(GIT_EOSERR, "Failed to create temporary file %s", path_out->ptr);
+	if ((fd = p_mkstemp(path_out->ptr)) < 0) {
+		giterr_set(GITERR_OS,
+			"Failed to create temporary file '%s': %s", path_out->ptr, strerror(errno));
+		return -1;
+	}
 
 	return fd;
 }
 
 int git_futils_creat_withpath(const char *path, const mode_t dirmode, const mode_t mode)
 {
-	if (git_futils_mkpath2file(path, dirmode) < GIT_SUCCESS)
-		return git__throw(GIT_EOSERR, "Failed to create file %s", path);
+	int fd;
 
-	return p_creat(path, mode);
+	if (git_futils_mkpath2file(path, dirmode) < 0)
+		return -1;
+
+	fd = p_creat(path, mode);
+	if (fd < 0) {
+		giterr_set(GITERR_OS,
+			"Failed to create file '%s': %s", path, strerror(errno));
+		return -1;
+	}
+
+	return fd;
 }
 
 int git_futils_creat_locked(const char *path, const mode_t mode)
 {
 	int fd = open(path, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_EXCL, mode);
-	return fd >= 0 ? fd : git__throw(GIT_EOSERR, "Failed to create locked file. Could not open %s", path);
+	if (fd < 0) {
+		giterr_set(GITERR_OS,
+			"Failed to create locked file '%s': %s", path, strerror(errno));
+		return -1;
+	}
+
+	return fd;
 }
 
 int git_futils_creat_locked_withpath(const char *path, const mode_t dirmode, const mode_t mode)
 {
-	if (git_futils_mkpath2file(path, dirmode) < GIT_SUCCESS)
-		return git__throw(GIT_EOSERR, "Failed to create locked file %s", path);
+	if (git_futils_mkpath2file(path, dirmode) < 0)
+		return -1;
 
 	return git_futils_creat_locked(path, mode);
 }
@@ -105,12 +116,14 @@ int git_futils_readbuffer_updated(git_buf *buf, const char *path, time_t *mtime,
 		*updated = 0;
 
 	if ((fd = p_open(path, O_RDONLY)) < 0) {
-		return git__throw(GIT_ENOTFOUND, "Failed to read file '%s': %s", path, strerror(errno));
+		giterr_set(GITERR_OS, "Failed to read file '%s': %s", path, strerror(errno));
+		return (errno == ENOENT) ? GIT_ENOTFOUND : -1;
 	}
 
 	if (p_fstat(fd, &st) < 0 || S_ISDIR(st.st_mode) || !git__is_sizet(st.st_size+1)) {
 		close(fd);
-		return git__throw(GIT_EOSERR, "Failed to stat file '%s'", path);
+		giterr_set(GITERR_OS, "Invalid regular file stat for '%s'", path);
+		return -1;
 	}
 
 	/*
@@ -141,7 +154,9 @@ int git_futils_readbuffer_updated(git_buf *buf, const char *path, time_t *mtime,
 
 		if (read_size < 0) {
 			close(fd);
-			return git__throw(GIT_EOSERR, "Failed to read from FD");
+			giterr_set(GITERR_OS, "Failed to read descriptor for %s: %s",
+				path, strerror(errno));
+			return -1;
 		}
 
 		len -= read_size;
@@ -218,7 +233,7 @@ int git_futils_mkdir_r(const char *path, const char *base, const mode_t mode)
 		pp += root_path_offset; /* On Windows, will skip the drive name (eg. C: or D:) */
 
 	while (error == GIT_SUCCESS && (sp = strchr(pp, '/')) != NULL) {
-		if (sp != pp && git_path_isdir(make_path.ptr) < GIT_SUCCESS) {
+		if (sp != pp && git_path_isdir(make_path.ptr) == false) {
 			*sp = 0;
 			error = p_mkdir(make_path.ptr, mode);
 
@@ -251,7 +266,7 @@ static int _rmdir_recurs_foreach(void *opaque, git_buf *path)
 	int error = GIT_SUCCESS;
 	int force = *(int *)opaque;
 
-	if (git_path_isdir(path->ptr) == GIT_SUCCESS) {
+	if (git_path_isdir(path->ptr) == true) {
 		error = git_path_direach(path, _rmdir_recurs_foreach, opaque);
 		if (error < GIT_SUCCESS)
 			return git__rethrow(error, "Failed to remove directory `%s`", path->ptr);
@@ -293,7 +308,7 @@ int git_futils_find_global_file(git_buf *path, const char *filename)
 	if ((error = git_buf_joinpath(path, home, filename)) < GIT_SUCCESS)
 		return error;
 
-	if (git_path_exists(path->ptr) < GIT_SUCCESS) {
+	if (git_path_exists(path->ptr) == false) {
 		git_buf_clear(path);
 		return GIT_ENOTFOUND;
 	}
@@ -395,7 +410,7 @@ 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_path_exists(path->ptr) == GIT_SUCCESS)
+	if (git_path_exists(path->ptr) == true)
 		return GIT_SUCCESS;
 
 	git_buf_clear(path);

src/global.h

@@ -14,6 +14,9 @@ typedef struct {
 		char last[1024];
 	} error;
 
+	git_error *last_error;
+	git_error error_t;
+
 	git_mwindow_ctl mem_ctl;
 } git_global_st;
 

src/index.c

@@ -150,7 +150,7 @@ int git_index_open(git_index **index_out, const char *index_path)
 	git_vector_init(&index->entries, 32, index_cmp);
 
 	/* Check if index file is stored on disk already */
-	if (git_path_exists(index->index_file_path) == 0)
+	if (git_path_exists(index->index_file_path) == true)
 		index->on_disk = 1;
 
 	*index_out = index;
@@ -221,7 +221,7 @@ int git_index_read(git_index *index)
 
 	assert(index->index_file_path);
 
-	if (!index->on_disk || git_path_exists(index->index_file_path) < 0) {
+	if (!index->on_disk || git_path_exists(index->index_file_path) == false) {
 		git_index_clear(index);
 		index->on_disk = 0;
 		return GIT_SUCCESS;

src/odb.c

@@ -402,7 +402,7 @@ static int load_alternates(git_odb *odb, const char *objects_dir)
 	if (error < GIT_SUCCESS)
 		return error;
 
-	if (git_path_exists(alternates_path.ptr) < GIT_SUCCESS) {
+	if (git_path_exists(alternates_path.ptr) == false) {
 		git_buf_free(&alternates_path);
 		return GIT_SUCCESS;
 	}

src/odb_loose.c

@@ -466,7 +466,7 @@ static int locate_object(
 	int error = object_file_name(object_location, backend->objects_dir, oid);
 
 	if (error == GIT_SUCCESS)
-		error = git_path_exists(git_buf_cstr(object_location));
+		error = git_path_exists(git_buf_cstr(object_location)) ? GIT_SUCCESS : GIT_ENOTFOUND;
 
 	return error;
 }
@@ -480,7 +480,7 @@ static int fn_locate_object_short_oid(void *state, git_buf *pathbuf) {
 		return GIT_SUCCESS;
 	}
 
-	if (!git_path_exists(pathbuf->ptr) && git_path_isdir(pathbuf->ptr)) {
+	if (git_path_isdir(pathbuf->ptr) == true) {
 		/* We are already in the directory matching the 2 first hex characters,
 		 * compare the first ncmp characters of the oids */
 		if (!memcmp(sstate->short_oid + 2,
@@ -533,8 +533,7 @@ static int locate_object_short_oid(
 		return git__rethrow(error, "Failed to locate object from short oid");
 
 	/* Check that directory exists */
-	if (git_path_exists(object_location->ptr) ||
-		git_path_isdir(object_location->ptr))
+	if (git_path_isdir(object_location->ptr) == false)
 		return git__throw(GIT_ENOTFOUND, "Failed to locate object from short oid. Object not found");
 
 	state.dir_len = object_location->size;
@@ -716,7 +715,7 @@ static int loose_backend__stream_fwrite(git_oid *oid, git_odb_stream *_stream)
 	 * is what git does and allows us to sidestep the fact that
 	 * we're not allowed to overwrite a read-only file on Windows.
 	 */
-	if (git_path_exists(final_path.ptr) == GIT_SUCCESS) {
+	if (git_path_exists(final_path.ptr) == true) {
 		git_filebuf_cleanup(&stream->fbuf);
 		goto cleanup;
 	}

src/odb_pack.c

@@ -469,7 +469,7 @@ int git_odb_backend_pack(git_odb_backend **backend_out, const char *objects_dir)
 	if (error < GIT_SUCCESS)
 		goto cleanup;
 
-	if (git_path_isdir(git_buf_cstr(&path)) == GIT_SUCCESS) {
+	if (git_path_isdir(git_buf_cstr(&path)) == true) {
 		backend->pack_folder = git_buf_detach(&path);
 		backend->pack_folder_mtime = 0;
 	}

src/pack.c

@@ -600,7 +600,7 @@ int git_packfile_check(struct git_pack_file **pack_out, const char *path)
 	memcpy(p->pack_name, path, path_len);
 
 	strcpy(p->pack_name + path_len, ".keep");
-	if (git_path_exists(p->pack_name) == GIT_SUCCESS)
+	if (git_path_exists(p->pack_name) == true)
 		p->pack_keep = 1;
 
 	strcpy(p->pack_name + path_len, ".pack");

src/path.c

@@ -354,80 +354,75 @@ int git_path_walk_up(
 	return error;
 }
 
-int git_path_exists(const char *path)
+bool git_path_exists(const char *path)
 {
 	assert(path);
-	return p_access(path, F_OK);
+	return p_access(path, F_OK) == 0;
 }
 
-int git_path_isdir(const char *path)
+bool git_path_isdir(const char *path)
 {
 #ifdef GIT_WIN32
 	DWORD attr = GetFileAttributes(path);
 	if (attr == INVALID_FILE_ATTRIBUTES)
-		return GIT_ERROR;
+		return false;
 
-	return (attr & FILE_ATTRIBUTE_DIRECTORY) ? GIT_SUCCESS : GIT_ERROR;
+	return (attr & FILE_ATTRIBUTE_DIRECTORY) != 0;
 
 #else
 	struct stat st;
-	if (p_stat(path, &st) < GIT_SUCCESS)
-		return GIT_ERROR;
+	if (p_stat(path, &st) < 0)
+		return false;
 
-	return S_ISDIR(st.st_mode) ? GIT_SUCCESS : GIT_ERROR;
+	return S_ISDIR(st.st_mode) != 0;
 #endif
 }
 
-int git_path_isfile(const char *path)
+bool git_path_isfile(const char *path)
 {
 	struct stat st;
-	int stat_error;
 
 	assert(path);
-	stat_error = p_stat(path, &st);
+	if (p_stat(path, &st) < 0)
+		return false;
 
-	if (stat_error < GIT_SUCCESS)
-		return -1;
-
-	if (!S_ISREG(st.st_mode))
-		return -1;
-
-	return 0;
+	return S_ISREG(st.st_mode) != 0;
 }
 
-static int _check_dir_contents(
+static bool _check_dir_contents(
 	git_buf *dir,
 	const char *sub,
-	int (*predicate)(const char *))
+	bool (*predicate)(const char *))
 {
-	int error = GIT_SUCCESS;
+	bool result;
 	size_t dir_size = dir->size;
 	size_t sub_size = strlen(sub);
 
-	/* separate allocation and join, so we can always leave git_buf valid */
-	if ((error = git_buf_try_grow(dir, dir_size + sub_size + 2)) < GIT_SUCCESS)
-		return error;
+	/* leave base valid even if we could not make space for subdir */
+	if (git_buf_try_grow(dir, dir_size + sub_size + 2) < 0)
+		return false;
+
+	/* save excursion */
 	git_buf_joinpath(dir, dir->ptr, sub);
 
-	error = (*predicate)(dir->ptr);
+	result = predicate(dir->ptr);
 
 	/* restore path */
 	git_buf_truncate(dir, dir_size);
-
-	return error;
+	return result;
 }
 
-int git_path_contains(git_buf *dir, const char *item)
+bool git_path_contains(git_buf *dir, const char *item)
 {
 	return _check_dir_contents(dir, item, &git_path_exists);
 }
 
-int git_path_contains_dir(git_buf *base, const char *subdir)
+bool git_path_contains_dir(git_buf *base, const char *subdir)
 {
 	return _check_dir_contents(base, subdir, &git_path_isdir);
 }
 
-int git_path_contains_file(git_buf *base, const char *file)
+bool git_path_contains_file(git_buf *base, const char *file)
 {
 	return _check_dir_contents(base, file, &git_path_isfile);
 }
@@ -448,7 +443,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) != GIT_SUCCESS)
+	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);
 
@@ -486,21 +481,20 @@ GIT_INLINE(int) is_dot_or_dotdot(const char *name)
 
 int git_path_direach(
 	git_buf *path,
-	int (*fn)(void *, git_buf *, git_error **),
-	void *arg,
-	git_error **error)
+	int (*fn)(void *, git_buf *),
+	void *arg)
 {
 	ssize_t wd_len;
 	DIR *dir;
 	struct dirent de_buf, *de;
 
-	if (git_path_to_dir(path, error) < 0)
+	if (git_path_to_dir(path) < 0)
 		return -1;
 
 	wd_len = path->size;
 	dir = opendir(path->ptr);
 	if (!dir) {
-		giterr_set(error, GITERR_OS, "Failed to `opendir` %s: %s", path->ptr, strerror(errno));
+		giterr_set(GITERR_OS, "Failed to `opendir` %s: %s", path->ptr, strerror(errno));
 		return -1;
 	}
 
@@ -510,12 +504,10 @@ int git_path_direach(
 		if (is_dot_or_dotdot(de->d_name))
 			continue;
 
-		if (git_buf_puts(path, de->d_name) < 0) {
-			giterr_set_oom(error);
+		if (git_buf_puts(path, de->d_name) < 0)
 			return -1;
-		}
 
-		result = fn(arg, path, error);
+		result = fn(arg, path);
 
 		git_buf_truncate(path, wd_len); /* restore path */
 
@@ -526,7 +518,7 @@ int git_path_direach(
 	}
 
 	closedir(dir);
-	return GIT_SUCCESS;
+	return 0;
 }
 
 int git_path_dirload(

src/path.h

@@ -113,21 +113,21 @@ extern int git_path_fromurl(git_buf *local_path_out, const char *file_url);
 
 /**
  * Check if a file exists and can be accessed.
- * @return GIT_SUCCESS if file exists, < 0 otherwise.
+ * @return true or false
  */
-extern int git_path_exists(const char *path);
+extern bool git_path_exists(const char *path);
 
 /**
  * Check if the given path points to a directory.
- * @return GIT_SUCCESS if it is a directory, < 0 otherwise.
+ * @return true or false
  */
-extern int git_path_isdir(const char *path);
+extern bool git_path_isdir(const char *path);
 
 /**
  * Check if the given path points to a regular file.
- * @return GIT_SUCCESS if it is a regular file, < 0 otherwise.
+ * @return true or false
  */
-extern int git_path_isfile(const char *path);
+extern bool git_path_isfile(const char *path);
 
 /**
  * Check if the parent directory contains the item.
@@ -136,25 +136,27 @@ extern int git_path_isfile(const char *path);
  * @param item Item that might be in the directory.
  * @return GIT_SUCCESS if item exists in directory, <0 otherwise.
  */
-extern int git_path_contains(git_buf *dir, const char *item);
+extern bool git_path_contains(git_buf *dir, const char *item);
 
 /**
  * Check if the given path contains the given subdirectory.
  *
  * @param parent Directory path that might contain subdir
  * @param subdir Subdirectory name to look for in parent
- * @return GIT_SUCCESS if subdirectory exists, < 0 otherwise.
+ * @param append_if_exists If true, then subdir will be appended to the parent path if it does exist
+ * @return true if subdirectory exists, false otherwise.
  */
-extern int git_path_contains_dir(git_buf *parent, const char *subdir);
+extern bool git_path_contains_dir(git_buf *parent, const char *subdir);
 
 /**
  * Check if the given path contains the given file.
  *
  * @param dir Directory path that might contain file
  * @param file File name to look for in parent
- * @return GIT_SUCCESS if file exists, < 0 otherwise.
+ * @param append_if_exists If true, then file will be appended to the path if it does exist
+ * @return true if file exists, false otherwise.
  */
-extern int git_path_contains_file(git_buf *dir, const char *file);
+extern bool git_path_contains_file(git_buf *dir, const char *file);
 
 /**
  * Clean up path, prepending base if it is not already rooted.

src/reflog.c

@@ -246,12 +246,12 @@ int git_reflog_write(git_reference *ref, const git_oid *oid_old,
 	if (error < GIT_SUCCESS)
 		goto cleanup;
 
-	if (git_path_exists(log_path.ptr)) {
+	if (git_path_exists(log_path.ptr) == false) {
 		error = git_futils_mkpath2file(log_path.ptr, GIT_REFLOG_DIR_MODE);
 		if (error < GIT_SUCCESS)
 			git__rethrow(error,
 				"Failed to write reflog. Cannot create reflog directory");
-	} else if (git_path_isfile(log_path.ptr)) {
+	} else if (git_path_isfile(log_path.ptr) == false) {
 		error = git__throw(GIT_ERROR,
 			"Failed to write reflog. `%s` is directory", log_path.ptr);
 	} else if (oid_old == NULL) {
@@ -302,7 +302,7 @@ int git_reflog_delete(git_reference *ref)
 	error = git_buf_join_n(&path, '/', 3,
 		ref->owner->path_repository, GIT_REFLOG_DIR, ref->name);
 
-	if (error == GIT_SUCCESS && git_path_exists(path.ptr) == 0)
+	if (error == GIT_SUCCESS && git_path_exists(path.ptr) == true)
 		error = p_unlink(path.ptr);
 
 	git_buf_free(&path);

src/repository.c

@@ -83,14 +83,14 @@ void git_repository_free(git_repository *repo)
 static int quickcheck_repository_dir(git_buf *repository_path)
 {
 	/* Check OBJECTS_DIR first, since it will generate the longest path name */
-	if (git_path_contains_dir(repository_path, GIT_OBJECTS_DIR) < 0)
+	if (git_path_contains_dir(repository_path, GIT_OBJECTS_DIR) == false)
 		return GIT_ERROR;
 
 	/* Ensure HEAD file exists */
-	if (git_path_contains_file(repository_path, GIT_HEAD_FILE) < 0)
+	if (git_path_contains_file(repository_path, GIT_HEAD_FILE) == false)
 		return GIT_ERROR;
 
-	if (git_path_contains_dir(repository_path, GIT_REFS_DIR) < 0)
+	if (git_path_contains_dir(repository_path, GIT_REFS_DIR)  == false)
 		return GIT_ERROR;
 
 	return GIT_SUCCESS;
@@ -498,7 +498,7 @@ static int read_gitfile(git_buf *path_out, const char *file_path, const char *ba
 
 	git_buf_free(&file);
 
-	if (error == GIT_SUCCESS && git_path_exists(path_out->ptr) == 0)
+	if (error == GIT_SUCCESS && git_path_exists(path_out->ptr) == true)
 		return GIT_SUCCESS;
 
 	return git__throw(GIT_EOBJCORRUPTED, "The `.git` file points to a nonexistent path");
@@ -542,7 +542,7 @@ int git_repository_discover(
 		 * If the `.git` file is regular instead of
 		 * a directory, it should contain the path of the actual git repository
 		 */
-		if (git_path_isfile(normal_path.ptr) == GIT_SUCCESS) {
+		if (git_path_isfile(normal_path.ptr) == true) {
 			git_buf gitfile_path = GIT_BUF_INIT;
 
 			error = read_gitfile(&gitfile_path, normal_path.ptr, bare_path.ptr);
@@ -564,7 +564,7 @@ int git_repository_discover(
 		/**
 		 * If the `.git` file is a folder, we check inside of it
 		 */
-		if (git_path_isdir(normal_path.ptr) == GIT_SUCCESS) {
+		if (git_path_isdir(normal_path.ptr) == true) {
 			error = quickcheck_repository_dir(&normal_path);
 			if (error == GIT_SUCCESS) {
 				found_path = &normal_path;
@@ -774,7 +774,7 @@ int git_repository_init(git_repository **repo_out, const char *path, unsigned is
 	if (error < GIT_SUCCESS)
 		return error;
 
-	if (git_path_isdir(repository_path.ptr) == GIT_SUCCESS) {
+	if (git_path_isdir(repository_path.ptr) == true) {
 		if (quickcheck_repository_dir(&repository_path) == GIT_SUCCESS) {
 			error = repo_init_reinit(repo_out, repository_path.ptr, is_bare);
 			git_buf_free(&repository_path);

src/status.c

@@ -505,7 +505,7 @@ int git_status_foreach(
 	dirent_st.index_position = 0;
 	dirent_st.is_dir = 1;
 
-	if (git_path_isdir(workdir)) {
+	if (git_path_isdir(workdir) == false) {
 		error = git__throw(GIT_EINVALIDPATH,
 			"Failed to determine status of file '%s'. "
 			"The given path doesn't lead to a folder", workdir);
@@ -608,7 +608,7 @@ int git_status_file(unsigned int *status_flags, git_repository *repo, const char
 		return git__rethrow(error,
 			"Failed to determine status of file '%s'", path);
 
-	if (git_path_isdir(temp_path.ptr) == GIT_SUCCESS) {
+	if (git_path_isdir(temp_path.ptr) == true) {
 		git_buf_free(&temp_path);
 		return git__throw(GIT_EINVALIDPATH,
 			"Failed to determine status of file '%s'. "
@@ -622,7 +622,7 @@ int git_status_file(unsigned int *status_flags, git_repository *repo, const char
 	}
 
 	/* Find file in Workdir */
-	if (git_path_exists(temp_path.ptr) == GIT_SUCCESS) {
+	if (git_path_exists(temp_path.ptr) == true) {
 		if ((error = status_entry_update_from_workdir(e, temp_path.ptr)) < GIT_SUCCESS)
 			goto cleanup;	/* The callee has already set the error message */
 	}
@@ -702,7 +702,7 @@ static char *alphasorted_dirent_info_new(const git_buf *path)
 
 	git_buf_copy_cstr(di, path->size + 1, path);
 
-	if (git_path_isdir(path->ptr) == GIT_SUCCESS) {
+	if (git_path_isdir(path->ptr) == true) {
 		/*
 		 * Append a forward slash to the name to force folders
 		 * to be ordered in a similar way than in a tree

tests-clar/config/stress.c

@@ -27,7 +27,7 @@ void test_config_stress__dont_break_on_invalid_input(void)
 	struct git_config_file *file;
 	git_config *config;
 
-	cl_git_pass(git_path_exists("git-test-config"));
+	cl_assert(git_path_exists("git-test-config"));
 	cl_git_pass(git_config_file__ondisk(&file, "git-test-config"));
 	cl_git_pass(git_config_new(&config));
 	cl_git_pass(git_config_add_file(config, file, 0));

tests-clar/core/filebuf.c

@@ -14,7 +14,7 @@ void test_core_filebuf__0(void)
 	cl_must_pass(p_close(fd));
 
 	cl_git_fail(git_filebuf_open(&file, test, 0));
-	cl_git_pass(git_path_exists(testlock));
+	cl_assert(git_path_exists(testlock));
 
 	cl_must_pass(p_unlink(testlock));
 }
@@ -56,20 +56,6 @@ void test_core_filebuf__2(void)
 	cl_must_pass(p_unlink(test));
 }
 
-
-/* make sure git_filebuf_open won't reopen an open buffer */
-void test_core_filebuf__3(void)
-{
-	git_filebuf file = GIT_FILEBUF_INIT;
-	char test[] = "test";
-
-	cl_git_pass(git_filebuf_open(&file, test, 0));
-	cl_git_fail(git_filebuf_open(&file, test, 0));
-
-	git_filebuf_cleanup(&file);
-}
-
-
 /* make sure git_filebuf_cleanup clears the buffer */
 void test_core_filebuf__4(void)
 {

tests/t00-core.c

@@ -485,7 +485,7 @@ BEGIN_TEST(filebuf0, "make sure git_filebuf_open doesn't delete an existing lock
 	must_pass(fd);
 	must_pass(p_close(fd));
 	must_fail(git_filebuf_open(&file, test, 0));
-	must_pass(git_path_exists(testlock));
+	must_be_true(git_path_exists(testlock));
 	must_pass(p_unlink(testlock));
 END_TEST
 

tests/t03-objwrite.c

@@ -44,9 +44,9 @@ static int make_odb_dir(void)
 
 static int check_object_files(object_data *d)
 {
-	if (git_path_exists(d->dir) < 0)
+	if (git_path_exists(d->dir) == false)
 		return -1;
-	if (git_path_exists(d->file) < 0)
+	if (git_path_exists(d->file) == false)
 		return -1;
 	return 0;
 }

tests/t08-tag.c

@@ -337,8 +337,6 @@ BEGIN_TEST(delete0, "Delete an already existing tag")
 	must_fail(git_reference_lookup(&ref_tag, repo, "refs/tags/e90810b"));
 
 	close_temp_repo(repo);
-
-	git_reference_free(ref_tag);
 END_TEST
 
 BEGIN_SUITE(tag)

tests/t10-refs.c

@@ -69,7 +69,6 @@ BEGIN_TEST(readtag1, "lookup a loose tag reference that doesn't exist")
 	must_fail(git_reference_lookup(&reference, repo, non_existing_tag_ref_name));
 
 	git_repository_free(repo);
-
 	git_reference_free(reference);
 END_TEST
 
@@ -530,7 +529,7 @@ BEGIN_TEST(pack1, "create a packfile from all the loose rn a repo")
 
 	/* Ensure the packed-refs file exists */
 	must_pass(git_buf_joinpath(&temp_path, repo->path_repository, GIT_PACKEDREFS_FILE));
-	must_pass(git_path_exists(temp_path.ptr));
+	must_be_true(git_path_exists(temp_path.ptr));
 
 	/* Ensure the known ref can still be looked up but is now packed */
 	must_pass(git_reference_lookup(&reference, repo, loose_tag_ref_name));
@@ -539,7 +538,7 @@ BEGIN_TEST(pack1, "create a packfile from all the loose rn a repo")
 
 	/* Ensure the known ref has been removed from the loose folder structure */
 	must_pass(git_buf_joinpath(&temp_path, repo->path_repository, loose_tag_ref_name));
-	must_pass(!git_path_exists(temp_path.ptr));
+	must_be_true(!git_path_exists(temp_path.ptr));
 
 	close_temp_repo(repo);
 
@@ -557,7 +556,7 @@ BEGIN_TEST(rename0, "rename a loose reference")
 
 	/* Ensure the ref doesn't exist on the file system */
 	must_pass(git_buf_joinpath(&temp_path, repo->path_repository, new_name));
-	must_pass(!git_path_exists(temp_path.ptr));
+	must_be_true(!git_path_exists(temp_path.ptr));
 
 	/* Retrieval of the reference to rename */
 	must_pass(git_reference_lookup(&looked_up_ref, repo, loose_tag_ref_name));
@@ -582,7 +581,7 @@ BEGIN_TEST(rename0, "rename a loose reference")
 
 	/* ...and the ref can be found in the file system */
 	must_pass(git_buf_joinpath(&temp_path, repo->path_repository, new_name));
-	must_pass(git_path_exists(temp_path.ptr));
+	must_be_true(git_path_exists(temp_path.ptr));
 
 	close_temp_repo(repo);
 
@@ -601,7 +600,7 @@ BEGIN_TEST(rename1, "rename a packed reference (should make it loose)")
 
 	/* Ensure the ref doesn't exist on the file system */
 	must_pass(git_buf_joinpath(&temp_path, repo->path_repository, packed_head_name));
-	must_pass(!git_path_exists(temp_path.ptr));
+	must_be_true(!git_path_exists(temp_path.ptr));
 
 	/* The reference can however be looked-up... */
 	must_pass(git_reference_lookup(&looked_up_ref, repo, packed_head_name));
@@ -626,7 +625,7 @@ BEGIN_TEST(rename1, "rename a packed reference (should make it loose)")
 
 	/* ...and the ref now happily lives in the file system */
 	must_pass(git_buf_joinpath(&temp_path, repo->path_repository, brand_new_name));
-	must_pass(git_path_exists(temp_path.ptr));
+	must_be_true(git_path_exists(temp_path.ptr));
 
 	close_temp_repo(repo);
 
@@ -645,7 +644,7 @@ BEGIN_TEST(rename2, "renaming a packed reference does not pack another reference
 
 	/* Ensure the other reference exists on the file system */
 	must_pass(git_buf_joinpath(&temp_path, repo->path_repository, packed_test_head_name));
-	must_pass(git_path_exists(temp_path.ptr));
+	must_be_true(git_path_exists(temp_path.ptr));
 
 	/* Lookup the other reference */
 	must_pass(git_reference_lookup(&another_looked_up_ref, repo, packed_test_head_name));
@@ -670,7 +669,7 @@ BEGIN_TEST(rename2, "renaming a packed reference does not pack another reference
 	must_be_true(git_reference_is_packed(another_looked_up_ref) == 0);
 
 	/* Ensure the other ref still exists on the file system */
-	must_pass(git_path_exists(temp_path.ptr));
+	must_be_true(git_path_exists(temp_path.ptr));
 
 	close_temp_repo(repo);
 
@@ -857,6 +856,18 @@ BEGIN_TEST(rename8, "can be renamed to a new name prefixed with the old name")
 END_TEST
 
 BEGIN_TEST(rename9, "can move a reference to a upper reference hierarchy")
+	/*
+	 * I'm killing this test because it adds a shitton of complexity
+	 * to the reference renaming code for an use case which I'd consider
+	 * "clinically stupid".
+	 *
+	 * If somebody shows me a workflow which involves turning a reference
+	 * into a folder with its same name, we'll bring back the ridiculous
+	 * logic.
+	 *
+	 * -Vicent
+	 */
+#if 0
     git_reference *ref, *ref_two, *looked_up_ref;
     git_repository *repo;
     git_oid id;
@@ -888,6 +899,7 @@ BEGIN_TEST(rename9, "can move a reference to a upper reference hierarchy")
     git_reference_free(looked_up_ref);
 
     close_temp_repo(repo);
+#endif
 END_TEST
 
 BEGIN_TEST(delete0, "deleting a ref which is both packed and loose should remove both tracks in the filesystem")
@@ -899,7 +911,7 @@ BEGIN_TEST(delete0, "deleting a ref which is both packed and loose should remove
 
 	/* Ensure the loose reference exists on the file system */
 	must_pass(git_buf_joinpath(&temp_path, repo->path_repository, packed_test_head_name));
-	must_pass(git_path_exists(temp_path.ptr));
+	must_be_true(git_path_exists(temp_path.ptr));
 
 	/* Lookup the reference */
 	must_pass(git_reference_lookup(&looked_up_ref, repo, packed_test_head_name));
@@ -914,7 +926,7 @@ BEGIN_TEST(delete0, "deleting a ref which is both packed and loose should remove
 	must_fail(git_reference_lookup(&another_looked_up_ref, repo, packed_test_head_name));
 
 	/* Ensure the loose reference doesn't exist any longer on the file system */
-	must_pass(!git_path_exists(temp_path.ptr));
+	must_be_true(!git_path_exists(temp_path.ptr));
 
 	close_temp_repo(repo);
 

tests/t12-repo.c

@@ -68,7 +68,7 @@ static int write_file(const char *path, const char *content)
 	int error;
 	git_file file;
 
-	if (git_path_exists(path) == GIT_SUCCESS) {
+	if (git_path_exists(path) == true) {
 		error = p_unlink(path);
 
 		if (error < GIT_SUCCESS)

tests/test_helpers.c

@@ -239,7 +239,7 @@ static int copy_filesystem_element_recurs(void *_data, git_buf *source)
 	git_buf_truncate(&data->dst, data->dst_baselen);
 	git_buf_puts(&data->dst, source->ptr + data->src_baselen);
 
-	if (git_path_isdir(source->ptr) == GIT_SUCCESS)
+	if (git_path_isdir(source->ptr) == true)
 		return git_path_direach(source, copy_filesystem_element_recurs, _data);
 	else
 		return copy_file(source->ptr, data->dst.ptr);
@@ -253,8 +253,8 @@ int copydir_recurs(
 	copydir_data data = { GIT_BUF_INIT, 0, GIT_BUF_INIT, 0 };
 
 	/* Source has to exist, Destination hast to _not_ exist */
-	if (git_path_isdir(source_directory_path) != GIT_SUCCESS ||
-		git_path_isdir(destination_directory_path) == GIT_SUCCESS)
+	if (git_path_isdir(source_directory_path) == false ||
+		git_path_isdir(destination_directory_path) == true)
 		return GIT_EINVALIDPATH;
 
 	git_buf_joinpath(&data.src, source_directory_path, "");
@@ -299,7 +299,7 @@ static int remove_placeholders_recurs(void *_data, git_buf *path)
 	remove_data *data = (remove_data *)_data;
 	size_t pathlen;
 
-	if (!git_path_isdir(path->ptr))
+	if (git_path_isdir(path->ptr) == true)
 		return git_path_direach(path, remove_placeholders_recurs, data);
 
 	pathlen = path->size;
@@ -322,7 +322,7 @@ int remove_placeholders(const char *directory_path, const char *filename)
 	remove_data data;
 	git_buf buffer = GIT_BUF_INIT;
 
-	if (git_path_isdir(directory_path))
+	if (git_path_isdir(directory_path) == false)
 		return GIT_EINVALIDPATH;
 
 	if ((error = git_buf_sets(&buffer, directory_path)) < GIT_SUCCESS)