Create git_diff_line and extend git_diff_hunk

Instead of having functions with so very many parameters to pass
hunk and line data, this takes the existing git_diff_hunk struct
and extends it with more hunk data, plus adds a git_diff_line.
Those structs are used to pass back hunk and line data instead of
the old APIs that took tons of parameters.

Some work that was previously only being done for git_diff_patch
creation (scanning the diff content for exact line counts) is now
done for all callbacks, but the performance difference should not
be noticable.

3 files changed, 147 insertions, 160 deletions

diff --git a/src/diff_patch.c b/src/diff_patch.c
index 951368200..cc49d68eb 100644
--- a/src/diff_patch.c
+++ b/src/diff_patch.c
@@ -12,23 +12,10 @@
 #include "diff_xdiff.h"
 #include "fileops.h"
 
-/* cached information about a single span in a diff */
-typedef struct diff_patch_line diff_patch_line;
-struct diff_patch_line {
-	const char *ptr;
-	size_t len;
-	size_t lines;
-	size_t oldno;
-	size_t newno;
-	char origin;
-};
-
 /* cached information about a hunk in a diff */
 typedef struct diff_patch_hunk diff_patch_hunk;
 struct diff_patch_hunk {
 	git_diff_hunk hunk;
-	char   header[128];
-	size_t header_len;
 	size_t line_start;
 	size_t line_count;
 };
@@ -42,8 +29,7 @@ struct git_patch {
 	git_diff_file_content nfile;
 	uint32_t flags;
 	git_array_t(diff_patch_hunk) hunks;
-	git_array_t(diff_patch_line) lines;
-	size_t oldno, newno;
+	git_array_t(git_diff_line)   lines;
 	size_t content_size, context_size, header_size;
 	git_pool flattened;
 };
@@ -57,7 +43,8 @@ enum {
 	GIT_DIFF_PATCH_FLATTENED   = (1 << 5),
 };
 
-static void diff_output_init(git_diff_output*, const git_diff_options*,
+static void diff_output_init(
+	git_diff_output*, const git_diff_options*,
 	git_diff_file_cb, git_diff_hunk_cb, git_diff_line_cb, void*);
 
 static void diff_output_to_patch(git_diff_output *, git_patch *);
@@ -81,7 +68,7 @@ static void diff_patch_init_common(git_patch *patch)
 	diff_patch_update_binary(patch);
 
 	if ((patch->delta->flags & GIT_DIFF_FLAG_BINARY) != 0)
-		patch->flags |= GIT_DIFF_PATCH_LOADED; /* set LOADED but not DIFFABLE */
+		patch->flags |= GIT_DIFF_PATCH_LOADED; /* LOADED but not DIFFABLE */
 
 	patch->flags |= GIT_DIFF_PATCH_INITIALIZED;
 
@@ -687,7 +674,7 @@ int git_patch_line_stats(
 	memset(totals, 0, sizeof(totals));
 
 	for (idx = 0; idx < git_array_size(patch->lines); ++idx) {
-		diff_patch_line *line = git_array_get(patch->lines, idx);
+		git_diff_line *line = git_array_get(patch->lines, idx);
 		if (!line)
 			continue;
 
@@ -721,8 +708,6 @@ static int diff_error_outofrange(const char *thing)
 
 int git_patch_get_hunk(
 	const git_diff_hunk **out,
-	const char **header,
-	size_t *header_len,
 	size_t *lines_in_hunk,
 	git_patch *patch,
 	size_t hunk_idx)
@@ -734,15 +719,11 @@ int git_patch_get_hunk(
 
 	if (!hunk) {
 		if (out) *out = NULL;
-		if (header) *header = NULL;
-		if (header_len) *header_len = 0;
 		if (lines_in_hunk) *lines_in_hunk = 0;
 		return diff_error_outofrange("hunk");
 	}
 
 	if (out) *out = &hunk->hunk;
-	if (header) *header = hunk->header;
-	if (header_len) *header_len = hunk->header_len;
 	if (lines_in_hunk) *lines_in_hunk = hunk->line_count;
 	return 0;
 }
@@ -758,49 +739,30 @@ int git_patch_num_lines_in_hunk(git_patch *patch, size_t hunk_idx)
 }
 
 int git_patch_get_line_in_hunk(
-	char *line_origin,
-	const char **content,
-	size_t *content_len,
-	int *old_lineno,
-	int *new_lineno,
+	const git_diff_line **out,
 	git_patch *patch,
 	size_t hunk_idx,
 	size_t line_of_hunk)
 {
 	diff_patch_hunk *hunk;
-	diff_patch_line *line;
-	const char *thing;
+	git_diff_line *line;
 
 	assert(patch);
 
 	if (!(hunk = git_array_get(patch->hunks, hunk_idx))) {
-		thing = "hunk";
-		goto notfound;
+		if (out) *out = NULL;
+		return diff_error_outofrange("hunk");
 	}
 
 	if (line_of_hunk >= hunk->line_count ||
 		!(line = git_array_get(
 			patch->lines, hunk->line_start + line_of_hunk))) {
-		thing = "line";
-		goto notfound;
+		if (out) *out = NULL;
+		return diff_error_outofrange("line");
 	}
 
-	if (line_origin) *line_origin = line->origin;
-	if (content) *content = line->ptr;
-	if (content_len) *content_len = line->len;
-	if (old_lineno) *old_lineno = (int)line->oldno;
-	if (new_lineno) *new_lineno = (int)line->newno;
-
+	if (out) *out = line;
 	return 0;
-
-notfound:
-	if (line_origin) *line_origin = GIT_DIFF_LINE_CONTEXT;
-	if (content) *content = NULL;
-	if (content_len) *content_len = 0;
-	if (old_lineno) *old_lineno = -1;
-	if (new_lineno) *new_lineno = -1;
-
-	return diff_error_outofrange(thing);
 }
 
 size_t git_patch_size(
@@ -880,18 +842,16 @@ int git_patch__invoke_callbacks(
 	for (i = 0; !error && i < git_array_size(patch->hunks); ++i) {
 		diff_patch_hunk *h = git_array_get(patch->hunks, i);
 
-		error = hunk_cb(
-			patch->delta, &h->hunk, h->header, h->header_len, payload);
+		error = hunk_cb(patch->delta, &h->hunk, payload);
 
 		if (!line_cb)
 			continue;
 
 		for (j = 0; !error && j < h->line_count; ++j) {
-			diff_patch_line *l =
+			git_diff_line *l =
 				git_array_get(patch->lines, h->line_start + j);
 
-			error = line_cb(
-				patch->delta, &h->hunk, l->origin, l->ptr, l->len, payload);
+			error = line_cb(patch->delta, &h->hunk, l, payload);
 		}
 	}
 
@@ -911,8 +871,6 @@ static int diff_patch_file_cb(
 static int diff_patch_hunk_cb(
 	const git_diff_delta *delta,
 	const git_diff_hunk *hunk_,
-	const char *header,
-	size_t header_len,
 	void *payload)
 {
 	git_patch *patch = payload;
@@ -925,34 +883,23 @@ static int diff_patch_hunk_cb(
 
 	memcpy(&hunk->hunk, hunk_, sizeof(hunk->hunk));
 
-	assert(header_len + 1 < sizeof(hunk->header));
-	memcpy(&hunk->header, header, header_len);
-	hunk->header[header_len] = '\0';
-	hunk->header_len = header_len;
-
-	patch->header_size += header_len;
+	patch->header_size += hunk_->header_len;
 
 	hunk->line_start = git_array_size(patch->lines);
 	hunk->line_count = 0;
 
-	patch->oldno = hunk_->old_start;
-	patch->newno = hunk_->new_start;
-
 	return 0;
 }
 
 static int diff_patch_line_cb(
 	const git_diff_delta *delta,
 	const git_diff_hunk *hunk_,
-	char line_origin,
-	const char *content,
-	size_t content_len,
+	const git_diff_line *line_,
 	void *payload)
 {
 	git_patch *patch = payload;
 	diff_patch_hunk *hunk;
-	diff_patch_line *line;
-	const char *content_end = content + content_len;
+	git_diff_line   *line;
 
 	GIT_UNUSED(delta);
 	GIT_UNUSED(hunk_);
@@ -963,48 +910,20 @@ static int diff_patch_line_cb(
 	line = git_array_alloc(patch->lines);
 	GITERR_CHECK_ALLOC(line);
 
-	line->ptr = content;
-	line->len = content_len;
-	line->origin = line_origin;
+	memcpy(line, line_, sizeof(*line));
 
 	/* do some bookkeeping so we can provide old/new line numbers */
 
-	line->lines = 0;
-	while (content < content_end)
-		if (*content++ == '\n')
-			++line->lines;
+	patch->content_size += line->content_len;
 
-	patch->content_size += content_len;
-
-	switch (line_origin) {
-	case GIT_DIFF_LINE_ADDITION:
-		patch->content_size += 1;
-	case GIT_DIFF_LINE_DEL_EOFNL:
-		line->oldno = -1;
-		line->newno = patch->newno;
-		patch->newno += line->lines;
-		break;
-	case GIT_DIFF_LINE_DELETION:
+	if (line->origin == GIT_DIFF_LINE_ADDITION ||
+		line->origin == GIT_DIFF_LINE_DELETION)
 		patch->content_size += 1;
-	case GIT_DIFF_LINE_ADD_EOFNL:
-		line->oldno = patch->oldno;
-		line->newno = -1;
-		patch->oldno += line->lines;
-		break;
-	case GIT_DIFF_LINE_CONTEXT:
+	else if (line->origin == GIT_DIFF_LINE_CONTEXT) {
 		patch->content_size += 1;
-		patch->context_size += 1;
-	case GIT_DIFF_LINE_CONTEXT_EOFNL:
-		patch->context_size += content_len;
-		line->oldno = patch->oldno;
-		line->newno = patch->newno;
-		patch->oldno += line->lines;
-		patch->newno += line->lines;
-		break;
-	default:
-		assert(false);
-		break;
-	}
+		patch->context_size += line->content_len + 1;
+	} else if (line->origin == GIT_DIFF_LINE_CONTEXT_EOFNL)
+		patch->context_size += line->content_len;
 
 	hunk->line_count++;
 
diff --git a/src/diff_print.c b/src/diff_print.c
index a6423d92b..b04b11515 100644
--- a/src/diff_print.c
+++ b/src/diff_print.c
@@ -17,6 +17,7 @@ typedef struct {
 	git_buf *buf;
 	uint32_t flags;
 	int oid_strlen;
+	git_diff_line line;
 } diff_print_info;
 
 static int diff_print_info_init(
@@ -51,6 +52,11 @@ static int diff_print_info_init(
 	else if (pi->oid_strlen > GIT_OID_HEXSZ + 1)
 		pi->oid_strlen = GIT_OID_HEXSZ + 1;
 
+	memset(&pi->line, 0, sizeof(pi->line));
+	pi->line.old_lineno = -1;
+	pi->line.new_lineno = -1;
+	pi->line.num_lines  = 1;
+
 	return 0;
 }
 
@@ -107,8 +113,11 @@ static int diff_print_one_name_only(
 		git_buf_putc(out, '\n'))
 		return -1;
 
-	if (pi->print_cb(delta, NULL, GIT_DIFF_LINE_FILE_HDR,
-			git_buf_cstr(out), git_buf_len(out), pi->payload))
+	pi->line.origin      = GIT_DIFF_LINE_FILE_HDR;
+	pi->line.content     = git_buf_cstr(out);
+	pi->line.content_len = git_buf_len(out);
+
+	if (pi->print_cb(delta, NULL, &pi->line, pi->payload))
 		return callback_error();
 
 	return 0;
@@ -149,8 +158,11 @@ static int diff_print_one_name_status(
 	if (git_buf_oom(out))
 		return -1;
 
-	if (pi->print_cb(delta, NULL, GIT_DIFF_LINE_FILE_HDR,
-			git_buf_cstr(out), git_buf_len(out), pi->payload))
+	pi->line.origin      = GIT_DIFF_LINE_FILE_HDR;
+	pi->line.content     = git_buf_cstr(out);
+	pi->line.content_len = git_buf_len(out);
+
+	if (pi->print_cb(delta, NULL, &pi->line, pi->payload))
 		return callback_error();
 
 	return 0;
@@ -192,8 +204,11 @@ static int diff_print_one_raw(
 	if (git_buf_oom(out))
 		return -1;
 
-	if (pi->print_cb(delta, NULL, GIT_DIFF_LINE_FILE_HDR,
-			git_buf_cstr(out), git_buf_len(out), pi->payload))
+	pi->line.origin      = GIT_DIFF_LINE_FILE_HDR;
+	pi->line.content     = git_buf_cstr(out);
+	pi->line.content_len = git_buf_len(out);
+
+	if (pi->print_cb(delta, NULL, &pi->line, pi->payload))
 		return callback_error();
 
 	return 0;
@@ -302,8 +317,11 @@ static int diff_print_patch_file(
 			pi->buf, delta, oldpfx, newpfx, pi->oid_strlen) < 0)
 		return -1;
 
-	if (pi->print_cb(delta, NULL, GIT_DIFF_LINE_FILE_HDR,
-			git_buf_cstr(pi->buf), git_buf_len(pi->buf), pi->payload))
+	pi->line.origin      = GIT_DIFF_LINE_FILE_HDR;
+	pi->line.content     = git_buf_cstr(pi->buf);
+	pi->line.content_len = git_buf_len(pi->buf);
+
+	if (pi->print_cb(delta, NULL, &pi->line, pi->payload))
 		return callback_error();
 
 	if ((delta->flags & GIT_DIFF_FLAG_BINARY) == 0)
@@ -316,8 +334,12 @@ static int diff_print_patch_file(
 			"Binary files %s%s and %s%s differ\n") < 0)
 		return -1;
 
-	if (pi->print_cb(delta, NULL, GIT_DIFF_LINE_BINARY,
-			git_buf_cstr(pi->buf), git_buf_len(pi->buf), pi->payload))
+	pi->line.origin      = GIT_DIFF_LINE_BINARY;
+	pi->line.content     = git_buf_cstr(pi->buf);
+	pi->line.content_len = git_buf_len(pi->buf);
+	pi->line.num_lines   = 1;
+
+	if (pi->print_cb(delta, NULL, &pi->line, pi->payload))
 		return callback_error();
 
 	return 0;
@@ -325,9 +347,7 @@ static int diff_print_patch_file(
 
 static int diff_print_patch_hunk(
 	const git_diff_delta *d,
-	const git_diff_hunk *r,
-	const char *header,
-	size_t header_len,
+	const git_diff_hunk *h,
 	void *data)
 {
 	diff_print_info *pi = data;
@@ -335,12 +355,11 @@ static int diff_print_patch_hunk(
 	if (S_ISDIR(d->new_file.mode))
 		return 0;
 
-	git_buf_clear(pi->buf);
-	if (git_buf_put(pi->buf, header, header_len) < 0)
-		return -1;
+	pi->line.origin      = GIT_DIFF_LINE_HUNK_HDR;
+	pi->line.content     = h->header;
+	pi->line.content_len = h->header_len;
 
-	if (pi->print_cb(d, r, GIT_DIFF_LINE_HUNK_HDR,
-			git_buf_cstr(pi->buf), git_buf_len(pi->buf), pi->payload))
+	if (pi->print_cb(d, h, &pi->line, pi->payload))
 		return callback_error();
 
 	return 0;
@@ -348,10 +367,8 @@ static int diff_print_patch_hunk(
 
 static int diff_print_patch_line(
 	const git_diff_delta *delta,
-	const git_diff_hunk *range,
-	char line_origin, /* GIT_DIFF_LINE value from above */
-	const char *content,
-	size_t content_len,
+	const git_diff_hunk *hunk,
+	const git_diff_line *line,
 	void *data)
 {
 	diff_print_info *pi = data;
@@ -359,21 +376,7 @@ static int diff_print_patch_line(
 	if (S_ISDIR(delta->new_file.mode))
 		return 0;
 
-	git_buf_clear(pi->buf);
-	git_buf_grow(pi->buf, content_len + 2);
-
-	if (line_origin == GIT_DIFF_LINE_ADDITION ||
-		line_origin == GIT_DIFF_LINE_DELETION ||
-		line_origin == GIT_DIFF_LINE_CONTEXT)
-		git_buf_putc(pi->buf, line_origin);
-
-	git_buf_put(pi->buf, content, content_len);
-
-	if (git_buf_oom(pi->buf))
-		return -1;
-
-	if (pi->print_cb(delta, range, line_origin,
-			git_buf_cstr(pi->buf), git_buf_len(pi->buf), pi->payload))
+	if (pi->print_cb(delta, hunk, line, pi->payload))
 		return callback_error();
 
 	return 0;
@@ -452,15 +455,19 @@ int git_patch_print(
 
 static int diff_print_to_buffer_cb(
 	const git_diff_delta *delta,
-	const git_diff_hunk *range,
-	char line_origin,
-	const char *content,
-	size_t content_len,
+	const git_diff_hunk *hunk,
+	const git_diff_line *line,
 	void *payload)
 {
 	git_buf *output = payload;
-	GIT_UNUSED(delta); GIT_UNUSED(range); GIT_UNUSED(line_origin);
-	return git_buf_put(output, content, content_len);
+	GIT_UNUSED(delta); GIT_UNUSED(hunk);
+
+	if (line->origin == GIT_DIFF_LINE_ADDITION ||
+		line->origin == GIT_DIFF_LINE_DELETION ||
+		line->origin == GIT_DIFF_LINE_CONTEXT)
+		git_buf_putc(output, line->origin);
+
+	return git_buf_put(output, line->content, line->content_len);
 }
 
 /* print a git_patch to a string buffer */
diff --git a/src/diff_xdiff.c b/src/diff_xdiff.c
index 972039753..a90f8d5d1 100644
--- a/src/diff_xdiff.c
+++ b/src/diff_xdiff.c
@@ -53,36 +53,94 @@ typedef struct {
 	git_xdiff_output *xo;
 	git_patch *patch;
 	git_diff_hunk hunk;
+	size_t old_lineno, new_lineno;
 } git_xdiff_info;
 
+static int diff_update_lines(
+	git_xdiff_info *info,
+	git_diff_line *line,
+	const char *content,
+	size_t content_len)
+{
+	const char *scan = content, *scan_end = content + content_len;
+
+	for (line->num_lines = 0; scan < scan_end; ++scan)
+		if (*scan == '\n')
+			++line->num_lines;
+
+	line->content     = content;
+	line->content_len = content_len;
+
+	/* expect " "/"-"/"+", then data */
+	switch (line->origin) {
+	case GIT_DIFF_LINE_ADDITION:
+	case GIT_DIFF_LINE_DEL_EOFNL:
+		line->old_lineno = -1;
+		line->new_lineno = info->new_lineno;
+		info->new_lineno += line->num_lines;
+		break;
+	case GIT_DIFF_LINE_DELETION:
+	case GIT_DIFF_LINE_ADD_EOFNL:
+		line->old_lineno = info->old_lineno;
+		line->new_lineno = -1;
+		info->old_lineno += line->num_lines;
+		break;
+	case GIT_DIFF_LINE_CONTEXT:
+	case GIT_DIFF_LINE_CONTEXT_EOFNL:
+		line->old_lineno = info->old_lineno;
+		line->new_lineno = info->new_lineno;
+		info->old_lineno += line->num_lines;
+		info->new_lineno += line->num_lines;
+		break;
+	default:
+		giterr_set(GITERR_INVALID, "Unknown diff line origin %02x",
+			(unsigned int)line->origin);
+		return -1;
+	}
+
+	return 0;
+}
+
 static int git_xdiff_cb(void *priv, mmbuffer_t *bufs, int len)
 {
 	git_xdiff_info *info = priv;
 	git_patch *patch = info->patch;
 	const git_diff_delta *delta = git_patch_get_delta(patch);
 	git_diff_output *output = &info->xo->output;
+	git_diff_line line;
 
 	if (len == 1) {
 		output->error = git_xdiff_parse_hunk(&info->hunk, bufs[0].ptr);
 		if (output->error < 0)
 			return output->error;
 
+		info->hunk.header_len = bufs[0].size;
+		if (info->hunk.header_len >= sizeof(info->hunk.header))
+			info->hunk.header_len = sizeof(info->hunk.header) - 1;
+		memcpy(info->hunk.header, bufs[0].ptr, info->hunk.header_len);
+		info->hunk.header[info->hunk.header_len] = '\0';
+
 		if (output->hunk_cb != NULL &&
-			output->hunk_cb(delta, &info->hunk,
-				bufs[0].ptr, bufs[0].size, output->payload))
+			output->hunk_cb(delta, &info->hunk, output->payload))
 			output->error = GIT_EUSER;
+
+		info->old_lineno = info->hunk.old_start;
+		info->new_lineno = info->hunk.new_start;
 	}
 
 	if (len == 2 || len == 3) {
 		/* expect " "/"-"/"+", then data */
-		char origin =
+		line.origin =
 			(*bufs[0].ptr == '+') ? GIT_DIFF_LINE_ADDITION :
 			(*bufs[0].ptr == '-') ? GIT_DIFF_LINE_DELETION :
 			GIT_DIFF_LINE_CONTEXT;
 
-		if (output->data_cb != NULL &&
-			output->data_cb(delta, &info->hunk,
-				origin, bufs[1].ptr, bufs[1].size, output->payload))
+		output->error = diff_update_lines(
+			info, &line, bufs[1].ptr, bufs[1].size);
+
+		if (!output->error &&
+			output->data_cb != NULL &&
+			output->data_cb(delta, &info->hunk, &line, output->payload))
 			output->error = GIT_EUSER;
 	}
 
@@ -92,14 +150,17 @@ static int git_xdiff_cb(void *priv, mmbuffer_t *bufs, int len)
 		 * If we have a '-' and a third buf, then we have removed a line
 		 * with out a newline but added a blank line, so ADD_EOFNL.
 		 */
-		char origin =
+		line.origin =
 			(*bufs[0].ptr == '+') ? GIT_DIFF_LINE_DEL_EOFNL :
 			(*bufs[0].ptr == '-') ? GIT_DIFF_LINE_ADD_EOFNL :
 			GIT_DIFF_LINE_CONTEXT_EOFNL;
 
-		if (output->data_cb != NULL &&
-			output->data_cb(delta, &info->hunk,
-				origin, bufs[2].ptr, bufs[2].size, output->payload))
+		output->error = diff_update_lines(
+			info, &line, bufs[2].ptr, bufs[2].size);
+
+		if (!output->error &&
+			output->data_cb != NULL &&
+			output->data_cb(delta, &info->hunk, &line, output->payload))
 			output->error = GIT_EUSER;
 	}