1 files changed, 116 insertions, 87 deletions

diff --git a/src/diff_output.c b/src/diff_output.c
index 88ccc9d45..13434beb9 100644
--- a/src/diff_output.c
+++ b/src/diff_output.c
@@ -52,8 +52,8 @@ static int parse_hunk_header(git_diff_range *range, const char *header)
 	return 0;
 }
 
-#define KNOWN_BINARY_FLAGS (GIT_DIFF_FILE_BINARY|GIT_DIFF_FILE_NOT_BINARY)
-#define NOT_BINARY_FLAGS   (GIT_DIFF_FILE_NOT_BINARY|GIT_DIFF_FILE_NO_DATA)
+#define KNOWN_BINARY_FLAGS (GIT_DIFF_FLAG_BINARY|GIT_DIFF_FLAG_NOT_BINARY)
+#define NOT_BINARY_FLAGS   (GIT_DIFF_FLAG_NOT_BINARY|GIT_DIFF_FLAG__NO_DATA)
 
 static int update_file_is_binary_by_attr(
 	git_repository *repo, git_diff_file *file)
@@ -68,9 +68,9 @@ static int update_file_is_binary_by_attr(
 		return -1;
 
 	if (GIT_ATTR_FALSE(value))
-		file->flags |= GIT_DIFF_FILE_BINARY;
+		file->flags |= GIT_DIFF_FLAG_BINARY;
 	else if (GIT_ATTR_TRUE(value))
-		file->flags |= GIT_DIFF_FILE_NOT_BINARY;
+		file->flags |= GIT_DIFF_FLAG_NOT_BINARY;
 	/* otherwise leave file->flags alone */
 
 	return 0;
@@ -78,15 +78,15 @@ static int update_file_is_binary_by_attr(
 
 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)
-		delta->binary = 1;
+	if ((delta->old_file.flags & GIT_DIFF_FLAG_BINARY) != 0 ||
+		(delta->new_file.flags & GIT_DIFF_FLAG_BINARY) != 0)
+		delta->flags |= GIT_DIFF_FLAG_BINARY;
 
 	else if ((delta->old_file.flags & NOT_BINARY_FLAGS) != 0 &&
 			 (delta->new_file.flags & NOT_BINARY_FLAGS) != 0)
-		delta->binary = 0;
+		delta->flags |= GIT_DIFF_FLAG_NOT_BINARY;
 
-	/* otherwise leave delta->binary value untouched */
+	/* otherwise leave delta->flags binary value untouched */
 }
 
 /* returns if we forced binary setting (and no further checks needed) */
@@ -95,24 +95,24 @@ static bool diff_delta_is_binary_forced(
 	git_diff_delta *delta)
 {
 	/* return true if binary-ness has already been settled */
-	if (delta->binary != -1)
+	if ((delta->flags & KNOWN_BINARY_FLAGS) != 0)
 		return true;
 
 	/* make sure files are conceivably mmap-able */
 	if ((git_off_t)((size_t)delta->old_file.size) != delta->old_file.size ||
 		(git_off_t)((size_t)delta->new_file.size) != delta->new_file.size)
 	{
-		delta->old_file.flags |= GIT_DIFF_FILE_BINARY;
-		delta->new_file.flags |= GIT_DIFF_FILE_BINARY;
-		delta->binary = 1;
+		delta->old_file.flags |= GIT_DIFF_FLAG_BINARY;
+		delta->new_file.flags |= GIT_DIFF_FLAG_BINARY;
+		delta->flags |= GIT_DIFF_FLAG_BINARY;
 		return true;
 	}
 
 	/* check if user is forcing us to text diff these files */
 	if (ctxt->opts && (ctxt->opts->flags & GIT_DIFF_FORCE_TEXT) != 0) {
-		delta->old_file.flags |= GIT_DIFF_FILE_NOT_BINARY;
-		delta->new_file.flags |= GIT_DIFF_FILE_NOT_BINARY;
-		delta->binary = 0;
+		delta->old_file.flags |= GIT_DIFF_FLAG_NOT_BINARY;
+		delta->new_file.flags |= GIT_DIFF_FLAG_NOT_BINARY;
+		delta->flags |= GIT_DIFF_FLAG_NOT_BINARY;
 		return true;
 	}
 
@@ -125,8 +125,6 @@ static int diff_delta_is_binary_by_attr(
 	int error = 0, mirror_new;
 	git_diff_delta *delta = patch->delta;
 
-	delta->binary = -1;
-
 	if (diff_delta_is_binary_forced(ctxt, delta))
 		return 0;
 
@@ -152,23 +150,21 @@ static int diff_delta_is_binary_by_content(
 	git_diff_file *file,
 	const git_map *map)
 {
+	const git_buf search = { map->data, 0, min(map->len, 4000) };
+
 	if (diff_delta_is_binary_forced(ctxt, delta))
 		return 0;
 
-	if ((file->flags & KNOWN_BINARY_FLAGS) == 0) {
-		const git_buf search = { map->data, 0, min(map->len, 4000) };
-
-		/* TODO: provide encoding / binary detection callbacks that can
-		 * be UTF-8 aware, etc.  For now, instead of trying to be smart,
-		 * let's just use the simple NUL-byte detection that core git uses.
-		 */
+	/* TODO: provide encoding / binary detection callbacks that can
+	 * be UTF-8 aware, etc.  For now, instead of trying to be smart,
+	 * let's just use the simple NUL-byte detection that core git uses.
+	 */
 
-		/* previously was: if (git_buf_text_is_binary(&search)) */
-		if (git_buf_text_contains_nul(&search))
-			file->flags |= GIT_DIFF_FILE_BINARY;
-		else
-			file->flags |= GIT_DIFF_FILE_NOT_BINARY;
-	}
+	/* previously was: if (git_buf_text_is_binary(&search)) */
+	if (git_buf_text_contains_nul(&search))
+		file->flags |= GIT_DIFF_FLAG_BINARY;
+	else
+		file->flags |= GIT_DIFF_FLAG_NOT_BINARY;
 
 	update_delta_is_binary(delta);
 
@@ -192,7 +188,7 @@ static int diff_delta_is_binary_by_size(
 	}
 
 	if (file->size > threshold)
-		file->flags |= GIT_DIFF_FILE_BINARY;
+		file->flags |= GIT_DIFF_FLAG_BINARY;
 
 	update_delta_is_binary(delta);
 
@@ -247,7 +243,7 @@ static int get_blob_content(
 		map->data = git_buf_detach(&content);
 		map->len = strlen(map->data);
 
-		file->flags |= GIT_DIFF_FILE_FREE_DATA;
+		file->flags |= GIT_DIFF_FLAG__FREE_DATA;
 		return 0;
 	}
 
@@ -270,7 +266,7 @@ static int get_blob_content(
 	/* if blob is too large to diff, mark as binary */
 	if ((error = diff_delta_is_binary_by_size(ctxt, delta, file)) < 0)
 		return error;
-	if (delta->binary == 1)
+	if ((delta->flags & GIT_DIFF_FLAG_BINARY) != 0)
 		return 0;
 
 	if (odb_obj != NULL) {
@@ -306,14 +302,14 @@ static int get_workdir_sm_content(
 		return error;
 
 	/* update OID if we didn't have it previously */
-	if ((file->flags & GIT_DIFF_FILE_VALID_OID) == 0) {
+	if ((file->flags & GIT_DIFF_FLAG_VALID_OID) == 0) {
 		const git_oid* sm_head;
 
 		if ((sm_head = git_submodule_wd_id(sm)) != NULL ||
 			(sm_head = git_submodule_head_id(sm)) != NULL)
 		{
 			git_oid_cpy(&file->oid, sm_head);
-			file->flags |= GIT_DIFF_FILE_VALID_OID;
+			file->flags |= GIT_DIFF_FLAG_VALID_OID;
 		}
 	}
 
@@ -329,7 +325,7 @@ static int get_workdir_sm_content(
 	map->data = git_buf_detach(&content);
 	map->len = strlen(map->data);
 
-	file->flags |= GIT_DIFF_FILE_FREE_DATA;
+	file->flags |= GIT_DIFF_FLAG__FREE_DATA;
 
 	return 0;
 }
@@ -356,8 +352,8 @@ static int get_workdir_content(
 	if (S_ISLNK(file->mode)) {
 		ssize_t alloc_len, read_len;
 
-		file->flags |= GIT_DIFF_FILE_FREE_DATA;
-		file->flags |= GIT_DIFF_FILE_BINARY;
+		file->flags |= GIT_DIFF_FLAG__FREE_DATA;
+		file->flags |= GIT_DIFF_FLAG_BINARY;
 
 		/* link path on disk could be UTF-16, so prepare a buffer that is
 		 * big enough to handle some UTF-8 data expansion
@@ -389,7 +385,7 @@ static int get_workdir_content(
 			file->size = git_futils_filesize(fd);
 
 		if ((error = diff_delta_is_binary_by_size(ctxt, delta, file)) < 0 ||
-			delta->binary == 1)
+			(delta->flags & GIT_DIFF_FLAG_BINARY) != 0)
 			goto close_and_cleanup;
 
 		error = git_filters_load(
@@ -402,7 +398,7 @@ static int get_workdir_content(
 				goto close_and_cleanup;
 
 			error = git_futils_mmap_ro(map, fd, 0, (size_t)file->size);
-			file->flags |= GIT_DIFF_FILE_UNMAP_DATA;
+			file->flags |= GIT_DIFF_FLAG__UNMAP_DATA;
 		} else {
 			git_buf raw = GIT_BUF_INIT, filtered = GIT_BUF_INIT;
 
@@ -412,7 +408,7 @@ static int get_workdir_content(
 				map->len  = git_buf_len(&filtered);
 				map->data = git_buf_detach(&filtered);
 
-				file->flags |= GIT_DIFF_FILE_FREE_DATA;
+				file->flags |= GIT_DIFF_FLAG__FREE_DATA;
 			}
 
 			git_buf_free(&raw);
@@ -425,11 +421,11 @@ close_and_cleanup:
 	}
 
 	/* once data is loaded, update OID if we didn't have it previously */
-	if (!error && (file->flags & GIT_DIFF_FILE_VALID_OID) == 0) {
+	if (!error && (file->flags & GIT_DIFF_FLAG_VALID_OID) == 0) {
 		error = git_odb_hash(
 			&file->oid, map->data, map->len, GIT_OBJ_BLOB);
 		if (!error)
-			file->flags |= GIT_DIFF_FILE_VALID_OID;
+			file->flags |= GIT_DIFF_FLAG_VALID_OID;
 	}
 
 	if (!error)
@@ -445,22 +441,22 @@ static void release_content(git_diff_file *file, git_map *map, git_blob *blob)
 	if (blob != NULL)
 		git_blob_free(blob);
 
-	if (file->flags & GIT_DIFF_FILE_FREE_DATA) {
+	if (file->flags & GIT_DIFF_FLAG__FREE_DATA) {
 		git__free(map->data);
 		map->data = "";
 		map->len  = 0;
-		file->flags &= ~GIT_DIFF_FILE_FREE_DATA;
+		file->flags &= ~GIT_DIFF_FLAG__FREE_DATA;
 	}
-	else if (file->flags & GIT_DIFF_FILE_UNMAP_DATA) {
+	else if (file->flags & GIT_DIFF_FLAG__UNMAP_DATA) {
 		git_futils_mmap_free(map);
 		map->data = "";
 		map->len  = 0;
-		file->flags &= ~GIT_DIFF_FILE_UNMAP_DATA;
+		file->flags &= ~GIT_DIFF_FLAG__UNMAP_DATA;
 	}
 }
 
 
-static void diff_context_init(
+static int diff_context_init(
 	diff_context *ctxt,
 	git_diff_list *diff,
 	git_repository *repo,
@@ -472,6 +468,12 @@ static void diff_context_init(
 {
 	memset(ctxt, 0, sizeof(diff_context));
 
+	if (!repo && diff)
+		repo = diff->repo;
+
+	if (!opts && diff)
+		opts = &diff->opts;
+
 	ctxt->repo = repo;
 	ctxt->diff = diff;
 	ctxt->opts = opts;
@@ -482,6 +484,8 @@ static void diff_context_init(
 	ctxt->error = 0;
 
 	setup_xdiff_options(ctxt->opts, &ctxt->xdiff_config, &ctxt->xdiff_params);
+
+	return 0;
 }
 
 static int diff_delta_file_callback(
@@ -555,7 +559,7 @@ static int diff_patch_load(
 	patch->new_data.len  = 0;
 	patch->new_blob      = NULL;
 
-	if (delta->binary == 1)
+	if ((delta->flags & GIT_DIFF_FLAG_BINARY) != 0)
 		goto cleanup;
 
 	if (!ctxt->hunk_cb &&
@@ -565,25 +569,25 @@ static int diff_patch_load(
 
 	switch (delta->status) {
 	case GIT_DELTA_ADDED:
-		delta->old_file.flags |= GIT_DIFF_FILE_NO_DATA;
+		delta->old_file.flags |= GIT_DIFF_FLAG__NO_DATA;
 		break;
 	case GIT_DELTA_DELETED:
-		delta->new_file.flags |= GIT_DIFF_FILE_NO_DATA;
+		delta->new_file.flags |= GIT_DIFF_FLAG__NO_DATA;
 		break;
 	case GIT_DELTA_MODIFIED:
 		break;
 	case GIT_DELTA_UNTRACKED:
-		delta->old_file.flags |= GIT_DIFF_FILE_NO_DATA;
+		delta->old_file.flags |= GIT_DIFF_FLAG__NO_DATA;
 		if ((ctxt->opts->flags & GIT_DIFF_INCLUDE_UNTRACKED_CONTENT) == 0)
-			delta->new_file.flags |= GIT_DIFF_FILE_NO_DATA;
+			delta->new_file.flags |= GIT_DIFF_FLAG__NO_DATA;
 		break;
 	default:
-		delta->new_file.flags |= GIT_DIFF_FILE_NO_DATA;
-		delta->old_file.flags |= GIT_DIFF_FILE_NO_DATA;
+		delta->new_file.flags |= GIT_DIFF_FLAG__NO_DATA;
+		delta->old_file.flags |= GIT_DIFF_FLAG__NO_DATA;
 		break;
 	}
 
-#define CHECK_UNMODIFIED (GIT_DIFF_FILE_NO_DATA | GIT_DIFF_FILE_VALID_OID)
+#define CHECK_UNMODIFIED (GIT_DIFF_FLAG__NO_DATA | GIT_DIFF_FLAG_VALID_OID)
 
 	check_if_unmodified =
 		(delta->old_file.flags & CHECK_UNMODIFIED) == 0 &&
@@ -594,41 +598,41 @@ static int diff_patch_load(
 	 * memory footprint during diff.
 	 */
 
-	if ((delta->old_file.flags & GIT_DIFF_FILE_NO_DATA) == 0 &&
+	if ((delta->old_file.flags & GIT_DIFF_FLAG__NO_DATA) == 0 &&
 		patch->old_src == GIT_ITERATOR_TYPE_WORKDIR) {
 		if ((error = get_workdir_content(
 				ctxt, delta, &delta->old_file, &patch->old_data)) < 0)
 			goto cleanup;
-		if (delta->binary == 1)
+		if ((delta->flags & GIT_DIFF_FLAG_BINARY) != 0)
 			goto cleanup;
 	}
 
-	if ((delta->new_file.flags & GIT_DIFF_FILE_NO_DATA) == 0 &&
+	if ((delta->new_file.flags & GIT_DIFF_FLAG__NO_DATA) == 0 &&
 		patch->new_src == GIT_ITERATOR_TYPE_WORKDIR) {
 		if ((error = get_workdir_content(
 				ctxt, delta, &delta->new_file, &patch->new_data)) < 0)
 			goto cleanup;
-		if (delta->binary == 1)
+		if ((delta->flags & GIT_DIFF_FLAG_BINARY) != 0)
 			goto cleanup;
 	}
 
-	if ((delta->old_file.flags & GIT_DIFF_FILE_NO_DATA) == 0 &&
+	if ((delta->old_file.flags & GIT_DIFF_FLAG__NO_DATA) == 0 &&
 		patch->old_src != GIT_ITERATOR_TYPE_WORKDIR) {
 		if ((error = get_blob_content(
 				ctxt, delta, &delta->old_file,
 				&patch->old_data, &patch->old_blob)) < 0)
 			goto cleanup;
-		if (delta->binary == 1)
+		if ((delta->flags & GIT_DIFF_FLAG_BINARY) != 0)
 			goto cleanup;
 	}
 
-	if ((delta->new_file.flags & GIT_DIFF_FILE_NO_DATA) == 0 &&
+	if ((delta->new_file.flags & GIT_DIFF_FLAG__NO_DATA) == 0 &&
 		patch->new_src != GIT_ITERATOR_TYPE_WORKDIR) {
 		if ((error = get_blob_content(
 				ctxt, delta, &delta->new_file,
 				&patch->new_data, &patch->new_blob)) < 0)
 			goto cleanup;
-		if (delta->binary == 1)
+		if ((delta->flags & GIT_DIFF_FLAG_BINARY) != 0)
 			goto cleanup;
 	}
 
@@ -646,13 +650,13 @@ static int diff_patch_load(
 	}
 
 cleanup:
-	if (delta->binary == -1)
+	if ((delta->flags & KNOWN_BINARY_FLAGS) == 0)
 		update_delta_is_binary(delta);
 
 	if (!error) {
 		patch->flags |= GIT_DIFF_PATCH_LOADED;
 
-		if (delta->binary != 1 &&
+		if ((delta->flags & GIT_DIFF_FLAG_BINARY) == 0 &&
 			delta->status != GIT_DELTA_UNMODIFIED &&
 			(patch->old_data.len || patch->new_data.len) &&
 			!git_oid_equal(&delta->old_file.oid, &delta->new_file.oid))
@@ -926,6 +930,15 @@ static int diff_patch_line_cb(
 	return 0;
 }
 
+static int diff_required(git_diff_list *diff, const char *action)
+{
+	if (!diff) {
+		giterr_set(GITERR_INVALID, "Must provide valid diff to %s", action);
+		return -1;
+	}
+
+	return 0;
+}
 
 int git_diff_foreach(
 	git_diff_list *diff,
@@ -939,9 +952,12 @@ int git_diff_foreach(
 	size_t idx;
 	git_diff_patch patch;
 
-	diff_context_init(
-		&ctxt, diff, diff->repo, &diff->opts,
-		file_cb, hunk_cb, data_cb, payload);
+	if (diff_required(diff, "git_diff_foreach") < 0)
+		return -1;
+
+	if (diff_context_init(
+			&ctxt, diff, NULL, NULL, file_cb, hunk_cb, data_cb, payload) < 0)
+		return -1;
 
 	diff_patch_init(&ctxt, &patch);
 
@@ -1138,7 +1154,7 @@ static int print_patch_file(
 		newpath = "/dev/null";
 	}
 
-	if (delta->binary != 1) {
+	if ((delta->flags & GIT_DIFF_FLAG_BINARY) == 0) {
 		git_buf_printf(pi->buf, "--- %s%s\n", oldpfx, oldpath);
 		git_buf_printf(pi->buf, "+++ %s%s\n", newpfx, newpath);
 	}
@@ -1153,7 +1169,7 @@ static int print_patch_file(
 		return GIT_EUSER;
 	}
 
-	if (delta->binary != 1)
+	if ((delta->flags & GIT_DIFF_FLAG_BINARY) == 0)
 		return 0;
 
 	git_buf_clear(pi->buf);
@@ -1268,7 +1284,7 @@ static void set_data_from_blob(
 		map->data  = (char *)git_blob_rawcontent(blob);
 	} else {
 		file->size = 0;
-		file->flags |= GIT_DIFF_FILE_NO_DATA;
+		file->flags |= GIT_DIFF_FLAG__NO_DATA;
 
 		map->len   = 0;
 		map->data  = "";
@@ -1283,7 +1299,7 @@ static void set_data_from_buffer(
 	map->len   = buffer_len;
 
 	if (!buffer) {
-		file->flags |= GIT_DIFF_FILE_NO_DATA;
+		file->flags |= GIT_DIFF_FLAG__NO_DATA;
 		map->data = NULL;
 	} else {
 		map->data = (char *)buffer;
@@ -1310,8 +1326,10 @@ static int diff_single_init(
 
 	memset(data, 0, sizeof(*data));
 
-	diff_context_init(
-		&data->ctxt, NULL, repo, opts, file_cb, hunk_cb, data_cb, payload);
+	if (diff_context_init(
+			&data->ctxt, NULL, repo, opts,
+			file_cb, hunk_cb, data_cb, payload) < 0)
+		return -1;
 
 	diff_patch_init(&data->ctxt, &data->patch);
 
@@ -1322,13 +1340,13 @@ static int diff_single_apply(diff_single_data *data)
 {
 	int error;
 	git_diff_delta *delta = &data->delta;
-	bool has_old = ((delta->old_file.flags & GIT_DIFF_FILE_NO_DATA) == 0);
-	bool has_new = ((delta->new_file.flags & GIT_DIFF_FILE_NO_DATA) == 0);
+	bool has_old = ((delta->old_file.flags & GIT_DIFF_FLAG__NO_DATA) == 0);
+	bool has_new = ((delta->new_file.flags & GIT_DIFF_FLAG__NO_DATA) == 0);
 
 	/* finish setting up fake git_diff_delta record and loaded data */
 
 	data->patch.delta = delta;
-	delta->binary = -1;
+	delta->flags = delta->flags & ~KNOWN_BINARY_FLAGS;
 
 	delta->status = has_new ?
 		(has_old ? GIT_DELTA_MODIFIED : GIT_DELTA_ADDED) :
@@ -1345,7 +1363,8 @@ static int diff_single_apply(diff_single_data *data)
 
 	data->patch.flags |= GIT_DIFF_PATCH_LOADED;
 
-	if (delta->binary != 1 && delta->status != GIT_DELTA_UNMODIFIED)
+	if ((delta->flags & GIT_DIFF_FLAG_BINARY) == 0 &&
+		delta->status != GIT_DELTA_UNMODIFIED)
 		data->patch.flags |= GIT_DIFF_PATCH_DIFFABLE;
 
 	/* do diffs */
@@ -1377,6 +1396,9 @@ int git_diff_blobs(
 		new_blob ? git_object_owner((const git_object *)new_blob) :
 		old_blob ? git_object_owner((const git_object *)old_blob) : NULL;
 
+	if (!repo) /* Hmm, given two NULL blobs, silently do no callbacks? */
+		return 0;
+
 	if ((error = diff_single_init(
 			&d, repo, options, file_cb, hunk_cb, data_cb, payload)) < 0)
 		return error;
@@ -1408,6 +1430,9 @@ int git_diff_blob_to_buffer(
 	git_repository *repo =
 		old_blob ? git_object_owner((const git_object *)old_blob) : NULL;
 
+	if (!repo && !buf) /* Hmm, given NULLs, silently do no callbacks? */
+		return 0;
+
 	if ((error = diff_single_init(
 			&d, repo, options, file_cb, hunk_cb, data_cb, payload)) < 0)
 		return error;
@@ -1456,11 +1481,19 @@ int git_diff_get_patch(
 
 	if (patch_ptr)
 		*patch_ptr = NULL;
+	if (delta_ptr)
+		*delta_ptr = NULL;
+
+	if (diff_required(diff, "git_diff_get_patch") < 0)
+		return -1;
+
+	if (diff_context_init(
+			&ctxt, diff, NULL, NULL,
+			NULL, diff_patch_hunk_cb, diff_patch_line_cb, NULL) < 0)
+		return -1;
 
 	delta = git_vector_get(&diff->deltas, idx);
 	if (!delta) {
-		if (delta_ptr)
-			*delta_ptr = NULL;
 		giterr_set(GITERR_INVALID, "Index out of range for delta in diff");
 		return GIT_ENOTFOUND;
 	}
@@ -1469,14 +1502,10 @@ int git_diff_get_patch(
 		*delta_ptr = delta;
 
 	if (!patch_ptr &&
-		(delta->binary != -1 ||
+		((delta->flags & KNOWN_BINARY_FLAGS) != 0 ||
 		 (diff->opts.flags & GIT_DIFF_SKIP_BINARY_CHECK) != 0))
 		return 0;
 
-	diff_context_init(
-		&ctxt, diff, diff->repo, &diff->opts,
-		NULL, diff_patch_hunk_cb, diff_patch_line_cb, NULL);
-
 	if (git_diff_delta__should_skip(ctxt.opts, delta))
 		return 0;