Merge pull request #659 from libgit2/development-merge

New-error-handling

1 files changed, 131 insertions, 111 deletions

diff --git a/src/diff_output.c b/src/diff_output.c
index 23f74d216..c380db996 100644
--- a/src/diff_output.c
+++ b/src/diff_output.c
@@ -19,9 +19,10 @@ typedef struct {
 	git_diff_list *diff;
 	void *cb_data;
 	git_diff_hunk_fn hunk_cb;
-	git_diff_line_fn line_cb;
+	git_diff_data_fn line_cb;
 	unsigned int index;
 	git_diff_delta *delta;
+	git_diff_range range;
 } diff_output_info;
 
 static int read_next_int(const char **str, int *value)
@@ -35,31 +36,41 @@ static int read_next_int(const char **str, int *value)
 		v = (v * 10) + (*scan - '0');
 	*str = scan;
 	*value = v;
-	return (digits > 0) ? GIT_SUCCESS : GIT_ENOTFOUND;
+	return (digits > 0) ? 0 : -1;
 }
 
 static int diff_output_cb(void *priv, mmbuffer_t *bufs, int len)
 {
-	int err = GIT_SUCCESS;
 	diff_output_info *info = priv;
 
 	if (len == 1 && info->hunk_cb) {
 		git_diff_range range = { -1, 0, -1, 0 };
+		const char *scan = bufs[0].ptr;
 
 		/* expect something of the form "@@ -%d[,%d] +%d[,%d] @@" */
-		if (bufs[0].ptr[0] == '@') {
-			const char *scan = bufs[0].ptr;
-			if (!(err = read_next_int(&scan, &range.old_start)) && *scan == ',')
-				err = read_next_int(&scan, &range.old_lines);
-			if (!err &&
-				!(err = read_next_int(&scan, &range.new_start)) && *scan == ',')
-				err = read_next_int(&scan, &range.new_lines);
-			if (!err && range.old_start >= 0 && range.new_start >= 0)
-				err = info->hunk_cb(
-					info->cb_data, info->delta, &range, bufs[0].ptr, bufs[0].size);
-		}
+		if (*scan != '@')
+			return -1;
+
+		if (read_next_int(&scan, &range.old_start) < 0)
+			return -1;
+		if (*scan == ',' && read_next_int(&scan, &range.old_lines) < 0)
+			return -1;
+
+		if (read_next_int(&scan, &range.new_start) < 0)
+			return -1;
+		if (*scan == ',' && read_next_int(&scan, &range.new_lines) < 0)
+			return -1;
+
+		if (range.old_start < 0 || range.new_start < 0)
+			return -1;
+
+		memcpy(&info->range, &range, sizeof(git_diff_range));
+
+		return info->hunk_cb(
+			info->cb_data, info->delta, &range, bufs[0].ptr, bufs[0].size);
 	}
-	else if ((len == 2 || len == 3) && info->line_cb) {
+
+	if ((len == 2 || len == 3) && info->line_cb) {
 		int origin;
 
 		/* expect " "/"-"/"+", then data, then maybe newline */
@@ -68,41 +79,43 @@ static int diff_output_cb(void *priv, mmbuffer_t *bufs, int len)
 			(*bufs[0].ptr == '-') ? GIT_DIFF_LINE_DELETION :
 			GIT_DIFF_LINE_CONTEXT;
 
-		err = info->line_cb(
-			info->cb_data, info->delta, origin, bufs[1].ptr, bufs[1].size);
+		if (info->line_cb(
+			info->cb_data, info->delta, &info->range, origin, bufs[1].ptr, bufs[1].size) < 0)
+			return -1;
 
 		/* deal with adding and removing newline at EOF */
-		if (err == GIT_SUCCESS && len == 3) {
+		if (len == 3) {
 			if (origin == GIT_DIFF_LINE_ADDITION)
 				origin = GIT_DIFF_LINE_ADD_EOFNL;
 			else
 				origin = GIT_DIFF_LINE_DEL_EOFNL;
 
-			err = info->line_cb(
-				info->cb_data, info->delta, origin, bufs[2].ptr, bufs[2].size);
+			return info->line_cb(
+				info->cb_data, info->delta, &info->range, origin, bufs[2].ptr, bufs[2].size);
 		}
 	}
 
-	return err;
+	return 0;
 }
 
 #define BINARY_DIFF_FLAGS (GIT_DIFF_FILE_BINARY|GIT_DIFF_FILE_NOT_BINARY)
 
-static int set_file_is_binary_by_attr(git_repository *repo, git_diff_file *file)
+static int update_file_is_binary_by_attr(git_repository *repo, git_diff_file *file)
 {
 	const char *value;
-	int error = git_attr_get(repo, file->path, "diff", &value);
-	if (error != GIT_SUCCESS)
-		return error;
+	if (git_attr_get(repo, 0, file->path, "diff", &value) < 0)
+		return -1;
+
 	if (GIT_ATTR_FALSE(value))
 		file->flags |= GIT_DIFF_FILE_BINARY;
 	else if (GIT_ATTR_TRUE(value))
 		file->flags |= GIT_DIFF_FILE_NOT_BINARY;
 	/* otherwise leave file->flags alone */
-	return error;
+
+	return 0;
 }
 
-static void set_delta_is_binary(git_diff_delta *delta)
+static void update_delta_is_binary(git_diff_delta *delta)
 {
 	if ((delta->old_file.flags & GIT_DIFF_FILE_BINARY) != 0 ||
 		(delta->new_file.flags & GIT_DIFF_FILE_BINARY) != 0)
@@ -117,7 +130,7 @@ static int file_is_binary_by_attr(
 	git_diff_list *diff,
 	git_diff_delta *delta)
 {
-	int error, mirror_new;
+	int error = 0, mirror_new;
 
 	delta->binary = -1;
 
@@ -128,7 +141,7 @@ static int file_is_binary_by_attr(
 		delta->old_file.flags |= GIT_DIFF_FILE_BINARY;
 		delta->new_file.flags |= GIT_DIFF_FILE_BINARY;
 		delta->binary = 1;
-		return GIT_SUCCESS;
+		return 0;
 	}
 
 	/* check if user is forcing us to text diff these files */
@@ -136,22 +149,21 @@ static int file_is_binary_by_attr(
 		delta->old_file.flags |= GIT_DIFF_FILE_NOT_BINARY;
 		delta->new_file.flags |= GIT_DIFF_FILE_NOT_BINARY;
 		delta->binary = 0;
-		return GIT_SUCCESS;
+		return 0;
 	}
 
 	/* check diff attribute +, -, or 0 */
-	error = set_file_is_binary_by_attr(diff->repo, &delta->old_file);
-	if (error != GIT_SUCCESS)
-		return error;
+	if (update_file_is_binary_by_attr(diff->repo, &delta->old_file) < 0)
+		return -1;
 
 	mirror_new = (delta->new_file.path == delta->old_file.path ||
 				  strcmp(delta->new_file.path, delta->old_file.path) == 0);
 	if (mirror_new)
 		delta->new_file.flags &= (delta->old_file.flags & BINARY_DIFF_FLAGS);
 	else
-		error = set_file_is_binary_by_attr(diff->repo, &delta->new_file);
+		error = update_file_is_binary_by_attr(diff->repo, &delta->new_file);
 
-	set_delta_is_binary(delta);
+	update_delta_is_binary(delta);
 
 	return error;
 }
@@ -191,11 +203,11 @@ static int file_is_binary_by_content(
 			delta->new_file.flags |= GIT_DIFF_FILE_NOT_BINARY;
 	}
 
-	set_delta_is_binary(delta);
+	update_delta_is_binary(delta);
 
 	/* TODO: if value != NULL, implement diff drivers */
 
-	return GIT_SUCCESS;
+	return 0;
 }
 
 static void setup_xdiff_options(
@@ -226,17 +238,15 @@ static int get_blob_content(
 	git_map *map,
 	git_blob **blob)
 {
-	int error;
-
 	if (git_oid_iszero(oid))
-		return GIT_SUCCESS;
+		return 0;
 
-	if ((error = git_blob_lookup(blob, repo, oid)) == GIT_SUCCESS) {
-		map->data = (void *)git_blob_rawcontent(*blob);
-		map->len  = git_blob_rawsize(*blob);
-	}
+	if (git_blob_lookup(blob, repo, oid) < 0)
+		return -1;
 
-	return error;
+	map->data = (void *)git_blob_rawcontent(*blob);
+	map->len  = git_blob_rawsize(*blob);
+	return 0;
 }
 
 static int get_workdir_content(
@@ -244,35 +254,33 @@ static int get_workdir_content(
 	git_diff_file *file,
 	git_map *map)
 {
-	git_buf full_path = GIT_BUF_INIT;
-	int error = git_buf_joinpath(
-		&full_path, git_repository_workdir(repo), file->path);
-	if (error != GIT_SUCCESS)
-		return error;
+	int error = 0;
+	git_buf path = GIT_BUF_INIT;
+
+	if (git_buf_joinpath(&path, git_repository_workdir(repo), file->path) < 0)
+		return -1;
 
 	if (S_ISLNK(file->mode)) {
+		ssize_t read_len;
+
 		file->flags |= GIT_DIFF_FILE_FREE_DATA;
 		file->flags |= GIT_DIFF_FILE_BINARY;
 
 		map->data = git__malloc((size_t)file->size + 1);
-		if (map->data == NULL)
-			error = GIT_ENOMEM;
-		else {
-			ssize_t read_len =
-				p_readlink(full_path.ptr, map->data, (size_t)file->size + 1);
-			if (read_len != (ssize_t)file->size)
-				error = git__throw(
-					GIT_EOSERR, "Failed to read symlink %s", file->path);
-			else
-				map->len = read_len;
-
-		}
+		GITERR_CHECK_ALLOC(map->data);
+
+		read_len = p_readlink(path.ptr, map->data, (size_t)file->size + 1);
+		if (read_len != (ssize_t)file->size) {
+			giterr_set(GITERR_OS, "Failed to read symlink '%s'", file->path);
+			error = -1;
+		} else
+			map->len = read_len;
 	}
 	else {
-		error = git_futils_mmap_ro_file(map, full_path.ptr);
+		error = git_futils_mmap_ro_file(map, path.ptr);
 		file->flags |= GIT_DIFF_FILE_UNMAP_DATA;
 	}
-	git_buf_free(&full_path);
+	git_buf_free(&path);
 	return error;
 }
 
@@ -298,9 +306,9 @@ int git_diff_foreach(
 	void *data,
 	git_diff_file_fn file_cb,
 	git_diff_hunk_fn hunk_cb,
-	git_diff_line_fn line_cb)
+	git_diff_data_fn line_cb)
 {
-	int error = GIT_SUCCESS;
+	int error = 0;
 	diff_output_info info;
 	git_diff_delta *delta;
 	xpparam_t    xdiff_params;
@@ -322,7 +330,8 @@ int git_diff_foreach(
 		git_map old_data, new_data;
 		mmfile_t old_xdiff_data, new_xdiff_data;
 
-		if (delta->status == GIT_DELTA_UNMODIFIED)
+		if (delta->status == GIT_DELTA_UNMODIFIED &&
+			(diff->opts.flags & GIT_DIFF_INCLUDE_UNMODIFIED) == 0)
 			continue;
 
 		if (delta->status == GIT_DELTA_IGNORED &&
@@ -333,8 +342,7 @@ int git_diff_foreach(
 			(diff->opts.flags & GIT_DIFF_INCLUDE_UNTRACKED) == 0)
 			continue;
 
-		error = file_is_binary_by_attr(diff, delta);
-		if (error < GIT_SUCCESS)
+		if ((error = file_is_binary_by_attr(diff, delta)) < 0)
 			goto cleanup;
 
 		old_data.data = "";
@@ -358,7 +366,8 @@ int git_diff_foreach(
 			else
 				error = get_blob_content(
 					diff->repo, &delta->old_file.oid, &old_data, &old_blob);
-			if (error != GIT_SUCCESS)
+
+			if (error < 0)
 				goto cleanup;
 		}
 
@@ -372,13 +381,15 @@ int git_diff_foreach(
 			else
 				error = get_blob_content(
 					diff->repo, &delta->new_file.oid, &new_data, &new_blob);
-			if (error != GIT_SUCCESS)
+
+			if (error < 0)
 				goto cleanup;
 
 			if ((delta->new_file.flags | GIT_DIFF_FILE_VALID_OID) == 0) {
 				error = git_odb_hash(
 					&delta->new_file.oid, new_data.data, new_data.len, GIT_OBJ_BLOB);
-				if (error != GIT_SUCCESS)
+
+				if (error < 0)
 					goto cleanup;
 
 				/* since we did not have the definitive oid, we may have
@@ -386,7 +397,8 @@ int git_diff_foreach(
 				 */
 				if (git_oid_cmp(&delta->old_file.oid, &delta->new_file.oid) == 0) {
 					delta->status = GIT_DELTA_UNMODIFIED;
-					goto cleanup;
+					if ((diff->opts.flags & GIT_DIFF_INCLUDE_UNMODIFIED) == 0)
+						goto cleanup;
 				}
 			}
 		}
@@ -397,7 +409,7 @@ int git_diff_foreach(
 		if (delta->binary == -1) {
 			error = file_is_binary_by_content(
 				diff, delta, &old_data, &new_data);
-			if (error < GIT_SUCCESS)
+			if (error < 0)
 				goto cleanup;
 		}
 
@@ -407,7 +419,7 @@ int git_diff_foreach(
 
 		if (file_cb != NULL) {
 			error = file_cb(data, delta, (float)info.index / diff->deltas.length);
-			if (error != GIT_SUCCESS)
+			if (error < 0)
 				goto cleanup;
 		}
 
@@ -434,7 +446,7 @@ cleanup:
 		release_content(&delta->old_file, &old_data, old_blob);
 		release_content(&delta->new_file, &new_data, new_blob);
 
-		if (error != GIT_SUCCESS)
+		if (error < 0)
 			break;
 	}
 
@@ -444,7 +456,7 @@ cleanup:
 
 typedef struct {
 	git_diff_list *diff;
-	git_diff_output_fn print_cb;
+	git_diff_data_fn print_cb;
 	void *cb_data;
 	git_buf *buf;
 } diff_print_info;
@@ -479,7 +491,7 @@ static int print_compact(void *data, git_diff_delta *delta, float progress)
 	}
 
 	if (!code)
-		return GIT_SUCCESS;
+		return 0;
 
 	old_suffix = pick_suffix(delta->old_file.mode);
 	new_suffix = pick_suffix(delta->new_file.mode);
@@ -499,16 +511,16 @@ 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_file.path);
 
-	if (git_buf_lasterror(pi->buf) != GIT_SUCCESS)
-		return git_buf_lasterror(pi->buf);
+	if (git_buf_oom(pi->buf))
+		return -1;
 
-	return pi->print_cb(pi->cb_data, GIT_DIFF_LINE_FILE_HDR, pi->buf->ptr);
+	return pi->print_cb(pi->cb_data, delta, NULL, GIT_DIFF_LINE_FILE_HDR, git_buf_cstr(pi->buf), git_buf_len(pi->buf));
 }
 
 int git_diff_print_compact(
 	git_diff_list *diff,
 	void *cb_data,
-	git_diff_output_fn print_cb)
+	git_diff_data_fn print_cb)
 {
 	int error;
 	git_buf buf = GIT_BUF_INIT;
@@ -551,32 +563,42 @@ 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 -1;
+
+	return 0;
 }
 
 static int print_patch_file(void *data, git_diff_delta *delta, float progress)
 {
-	int error;
 	diff_print_info *pi = data;
 	const char *oldpfx = pi->diff->opts.old_prefix;
 	const char *oldpath = delta->old_file.path;
 	const char *newpfx = pi->diff->opts.new_prefix;
 	const char *newpath = delta->new_file.path;
+	int result;
 
 	GIT_UNUSED(progress);
 
+	if (!oldpfx)
+		oldpfx = DIFF_OLD_PREFIX_DEFAULT;
+
+	if (!newpfx)
+		newpfx = DIFF_NEW_PREFIX_DEFAULT;
+
 	git_buf_clear(pi->buf);
 	git_buf_printf(pi->buf, "diff --git %s%s %s%s\n", oldpfx, delta->old_file.path, newpfx, delta->new_file.path);
-	if ((error = print_oid_range(pi, delta)) < GIT_SUCCESS)
-		return error;
+
+	if (print_oid_range(pi, delta) < 0)
+		return -1;
 
 	if (git_oid_iszero(&delta->old_file.oid)) {
 		oldpfx = "";
 		oldpath = "/dev/null";
 	}
 	if (git_oid_iszero(&delta->new_file.oid)) {
-		oldpfx = "";
-		oldpath = "/dev/null";
+		newpfx = "";
+		newpath = "/dev/null";
 	}
 
 	if (delta->binary != 1) {
@@ -584,21 +606,24 @@ 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 -1;
 
-	error = pi->print_cb(pi->cb_data, GIT_DIFF_LINE_FILE_HDR, pi->buf->ptr);
-	if (error != GIT_SUCCESS || delta->binary != 1)
-		return error;
+    result = pi->print_cb(pi->cb_data, delta, NULL, GIT_DIFF_LINE_FILE_HDR, git_buf_cstr(pi->buf), git_buf_len(pi->buf));
+    if (result < 0)
+        return result;
+
+    if (delta->binary != 1)
+        return 0;
 
 	git_buf_clear(pi->buf);
 	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 -1;
 
-	return pi->print_cb(pi->cb_data, GIT_DIFF_LINE_BINARY, pi->buf->ptr);
+	return pi->print_cb(pi->cb_data, delta, NULL, GIT_DIFF_LINE_BINARY, git_buf_cstr(pi->buf), git_buf_len(pi->buf));
 }
 
 static int print_patch_hunk(
@@ -610,28 +635,23 @@ static int print_patch_hunk(
 {
 	diff_print_info *pi = data;
 
-	GIT_UNUSED(d);
-	GIT_UNUSED(r);
-
 	git_buf_clear(pi->buf);
+	if (git_buf_printf(pi->buf, "%.*s", (int)header_len, header) < 0)
+		return -1;
 
-	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 pi->print_cb(pi->cb_data, d, r, GIT_DIFF_LINE_HUNK_HDR, git_buf_cstr(pi->buf), git_buf_len(pi->buf));
 }
 
 static int print_patch_line(
 	void *data,
 	git_diff_delta *delta,
+	git_diff_range *range,
 	char line_origin, /* GIT_DIFF_LINE value from above */
 	const char *content,
 	size_t content_len)
 {
 	diff_print_info *pi = data;
 
-	GIT_UNUSED(delta);
-
 	git_buf_clear(pi->buf);
 
 	if (line_origin == GIT_DIFF_LINE_ADDITION ||
@@ -641,16 +661,16 @@ 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 -1;
 
-	return pi->print_cb(pi->cb_data, line_origin, pi->buf->ptr);
+	return pi->print_cb(pi->cb_data, delta, range, line_origin, git_buf_cstr(pi->buf), git_buf_len(pi->buf));
 }
 
 int git_diff_print_patch(
 	git_diff_list *diff,
 	void *cb_data,
-	git_diff_output_fn print_cb)
+	git_diff_data_fn print_cb)
 {
 	int error;
 	git_buf buf = GIT_BUF_INIT;
@@ -676,7 +696,7 @@ int git_diff_blobs(
 	git_diff_options *options,
 	void *cb_data,
 	git_diff_hunk_fn hunk_cb,
-	git_diff_line_fn line_cb)
+	git_diff_data_fn line_cb)
 {
 	diff_output_info info;
 	git_diff_delta delta;
@@ -732,5 +752,5 @@ int git_diff_blobs(
 
 	xdl_diff(&old_data, &new_data, &xdiff_params, &xdiff_config, &xdiff_callback);
 
-	return GIT_SUCCESS;
+	return 0;
 }