[PATCH 1/2] xdiff-interface: allow consume function to quit early by returning non-zero

[PATCH 1/2] xdiff-interface: allow consume function to quit early by returning non-zero

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 combine-diff.c     |    9 +++++----
 diff.c             |   32 +++++++++++++++++++-------------
 diffcore-pickaxe.c |    7 ++++---
 xdiff-interface.c  |   15 ++++++++++-----
 xdiff-interface.h  |    2 +-
 5 files changed, 39 insertions(+), 26 deletions(-)

diff --git a/combine-diff.c b/combine-diff.c
index be67cfc..d99e1c6 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -163,14 +163,14 @@ struct combine_diff_state {
 	struct sline *lost_bucket;
 };
 
-static void consume_line(void *state_, char *line, unsigned long len)
+static int consume_line(void *state_, char *line, unsigned long len)
 {
 	struct combine_diff_state *state = state_;
 	if (5 < len && !memcmp("@@ -", line, 4)) {
 		if (parse_hunk_header(line, len,
 				      &state->ob, &state->on,
 				      &state->nb, &state->nn))
-			return;
+			return 0;
 		state->lno = state->nb;
 		if (state->nn == 0) {
 			/* @@ -X,Y +N,0 @@ removed Y lines
@@ -194,10 +194,10 @@ static void consume_line(void *state_, char *line, unsigned long len)
 					sizeof(unsigned long));
 		state->sline[state->nb-1].p_lno[state->n] = state->ob;
 		state->lost_bucket->next_lost = state->lost_bucket->lost_head;
-		return;
+		return 0;
 	}
 	if (!state->lost_bucket)
-		return; /* not in any hunk yet */
+		return 0; /* not in any hunk yet */
 	switch (line[0]) {
 	case '-':
 		append_lost(state->lost_bucket, state->n, line+1, len-1);
@@ -207,6 +207,7 @@ static void consume_line(void *state_, char *line, unsigned long len)
 		state->lno++;
 		break;
 	}
+	return 0;
 }
 
 static void combine_diff(const unsigned char *parent, unsigned int mode,
diff --git a/diff.c b/diff.c
index 93ef9a2..2801204 100644
--- a/diff.c
+++ b/diff.c
@@ -788,7 +788,7 @@ static int color_words_output_graph_prefix(struct diff_words_data *diff_words)
 	}
 }
 
-static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len)
+static int fn_out_diff_words_aux(void *priv, char *line, unsigned long len)
 {
 	struct diff_words_data *diff_words = priv;
 	struct diff_words_style *style = diff_words->style;
@@ -800,7 +800,7 @@ static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len)
 
 	if (line[0] != '@' || parse_hunk_header(line, len,
 			&minus_first, &minus_len, &plus_first, &plus_len))
-		return;
+		return 0;
 
 	assert(opt);
 	if (opt->output_prefix) {
@@ -849,6 +849,7 @@ static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len)
 
 	diff_words->current_plus = plus_end;
 	diff_words->last_minus = minus_first;
+	return 0;
 }
 
 /* This function starts looking at *begin, and returns 0 iff a word was found. */
@@ -1042,7 +1043,7 @@ static void find_lno(const char *line, struct emit_callback *ecbdata)
 	ecbdata->lno_in_postimage = strtol(p + 1, NULL, 10);
 }
 
-static void fn_out_consume(void *priv, char *line, unsigned long len)
+static int fn_out_consume(void *priv, char *line, unsigned long len)
 {
 	struct emit_callback *ecbdata = priv;
 	const char *meta = diff_get_color(ecbdata->color_diff, DIFF_METAINFO);
@@ -1091,7 +1092,7 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
 		emit_hunk_header(ecbdata, line, len);
 		if (line[len-1] != '\n')
 			putc('\n', ecbdata->opt->file);
-		return;
+		return 0;
 	}
 
 	if (len < 1) {
@@ -1099,18 +1100,18 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
 		if (ecbdata->diff_words
 		    && ecbdata->diff_words->type == DIFF_WORDS_PORCELAIN)
 			fputs("~\n", ecbdata->opt->file);
-		return;
+		return 0;
 	}
 
 	if (ecbdata->diff_words) {
 		if (line[0] == '-') {
 			diff_words_append(line, len,
 					  &ecbdata->diff_words->minus);
-			return;
+			return 0;
 		} else if (line[0] == '+') {
 			diff_words_append(line, len,
 					  &ecbdata->diff_words->plus);
-			return;
+			return 0;
 		}
 		diff_words_flush(ecbdata);
 		if (ecbdata->diff_words->type == DIFF_WORDS_PORCELAIN) {
@@ -1128,7 +1129,7 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
 			}
 			emit_line(ecbdata->opt, plain, reset, line, len);
 		}
-		return;
+		return 0;
 	}
 
 	if (line[0] != '+') {
@@ -1143,6 +1144,8 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
 		ecbdata->lno_in_postimage++;
 		emit_add_line(reset, ecbdata, line + 1, len - 1);
 	}
+
+	return 0;
 }
 
 static char *pprint_rename(const char *a, const char *b)
@@ -1250,7 +1253,7 @@ static struct diffstat_file *diffstat_add(struct diffstat_t *diffstat,
 	return x;
 }
 
-static void diffstat_consume(void *priv, char *line, unsigned long len)
+static int diffstat_consume(void *priv, char *line, unsigned long len)
 {
 	struct diffstat_t *diffstat = priv;
 	struct diffstat_file *x = diffstat->files[diffstat->nr - 1];
@@ -1259,6 +1262,7 @@ static void diffstat_consume(void *priv, char *line, unsigned long len)
 		x->added++;
 	else if (line[0] == '-')
 		x->deleted++;
+	return 0;
 }
 
 const char mime_boundary_leader[] = "------------";
@@ -1805,7 +1809,7 @@ static int is_conflict_marker(const char *line, int marker_size, unsigned long l
 	return 1;
 }
 
-static void checkdiff_consume(void *priv, char *line, unsigned long len)
+static int checkdiff_consume(void *priv, char *line, unsigned long len)
 {
 	struct checkdiff_t *data = priv;
 	int color_diff = DIFF_OPT_TST(data->o, COLOR_DIFF);
@@ -1835,7 +1839,7 @@ static void checkdiff_consume(void *priv, char *line, unsigned long len)
 		}
 		bad = ws_check(line + 1, len - 1, data->ws_rule);
 		if (!bad)
-			return;
+			return 0;
 		data->status |= bad;
 		err = whitespace_error_string(bad);
 		fprintf(data->o->file, "%s%s:%d: %s.\n",
@@ -1853,6 +1857,7 @@ static void checkdiff_consume(void *priv, char *line, unsigned long len)
 		else
 			die("invalid diff");
 	}
+	return 0;
 }
 
 static unsigned char *deflate_it(char *data,
@@ -4008,19 +4013,20 @@ static int remove_space(char *line, int len)
 	return dst - line;
 }
 
-static void patch_id_consume(void *priv, char *line, unsigned long len)
+static int patch_id_consume(void *priv, char *line, unsigned long len)
 {
 	struct patch_id_t *data = priv;
 	int new_len;
 
 	/* Ignore line numbers when computing the SHA1 of the patch */
 	if (!prefixcmp(line, "@@ -"))
-		return;
+		return 0;
 
 	new_len = remove_space(line, len);
 
 	git_SHA1_Update(data->ctx, line, new_len);
 	data->patchlen += new_len;
+	return 0;
 }
 
 /* returns 0 upon success, and writes result into sha1 */
diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c
index ea03b91..12811b9 100644
--- a/diffcore-pickaxe.c
+++ b/diffcore-pickaxe.c
@@ -12,25 +12,26 @@ struct diffgrep_cb {
 	int hit;
 };
 
-static void diffgrep_consume(void *priv, char *line, unsigned long len)
+static int diffgrep_consume(void *priv, char *line, unsigned long len)
 {
 	struct diffgrep_cb *data = priv;
 	regmatch_t regmatch;
 	int hold;
 
 	if (line[0] != '+' && line[0] != '-')
-		return;
+		return 0;
 	if (data->hit)
 		/*
 		 * NEEDSWORK: we should have a way to terminate the
 		 * caller early.
 		 */
-		return;
+		return 0;
 	/* Yuck -- line ought to be "const char *"! */
 	hold = line[len];
 	line[len] = '\0';
 	data->hit = !regexec(data->regexp, line + 1, 1, &regmatch, 0);
 	line[len] = hold;
+	return 0;
 }
 
 static void fill_one(struct diff_filespec *one,
diff --git a/xdiff-interface.c b/xdiff-interface.c
index 0e2c169..c5684b4 100644
--- a/xdiff-interface.c
+++ b/xdiff-interface.c
@@ -56,7 +56,7 @@ int parse_hunk_header(char *line, int len,
 	return -!!memcmp(cp, " @@", 3);
 }
 
-static void consume_one(void *priv_, char *s, unsigned long size)
+static int consume_one(void *priv_, char *s, unsigned long size)
 {
 	struct xdiff_emit_state *priv = priv_;
 	char *ep;
@@ -64,10 +64,12 @@ static void consume_one(void *priv_, char *s, unsigned long size)
 		unsigned long this_size;
 		ep = memchr(s, '\n', size);
 		this_size = (ep == NULL) ? size : (ep - s + 1);
-		priv->consume(priv->consume_callback_data, s, this_size);
+		if (priv->consume(priv->consume_callback_data, s, this_size))
+			return -1;
 		size -= this_size;
 		s += this_size;
 	}
+	return 0;
 }
 
 static int xdiff_outf(void *priv_, mmbuffer_t *mb, int nbuf)
@@ -84,15 +86,18 @@ static int xdiff_outf(void *priv_, mmbuffer_t *mb, int nbuf)
 
 		/* we have a complete line */
 		if (!priv->remainder.len) {
-			consume_one(priv, mb[i].ptr, mb[i].size);
+			if (consume_one(priv, mb[i].ptr, mb[i].size))
+				return -1;
 			continue;
 		}
 		strbuf_add(&priv->remainder, mb[i].ptr, mb[i].size);
-		consume_one(priv, priv->remainder.buf, priv->remainder.len);
+		if (consume_one(priv, priv->remainder.buf, priv->remainder.len))
+			return -1;
 		strbuf_reset(&priv->remainder);
 	}
 	if (priv->remainder.len) {
-		consume_one(priv, priv->remainder.buf, priv->remainder.len);
+		if (consume_one(priv, priv->remainder.buf, priv->remainder.len))
+			return -1;
 		strbuf_reset(&priv->remainder);
 	}
 	return 0;
diff --git a/xdiff-interface.h b/xdiff-interface.h
index 49d1116..b7aaa0e 100644
--- a/xdiff-interface.h
+++ b/xdiff-interface.h
@@ -3,7 +3,7 @@
 
 #include "xdiff/xdiff.h"
 
-typedef void (*xdiff_emit_consume_fn)(void *, char *, unsigned long);
+typedef int (*xdiff_emit_consume_fn)(void *, char *, unsigned long);
 typedef void (*xdiff_emit_hunk_consume_fn)(void *, long, long, long);
 
 int xdi_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, xdemitconf_t const *xecfg, xdemitcb_t *ecb);
-- 
1.7.4.74.g639db

[PATCH 2/2] diffcore-pickaxe: terminate grepping as soon as it hits

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 diffcore-pickaxe.c |    6 +-----
 1 files changed, 1 insertions(+), 5 deletions(-)

diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c
index 12811b9..e028dd5 100644
--- a/diffcore-pickaxe.c
+++ b/diffcore-pickaxe.c
@@ -21,11 +21,7 @@ static int diffgrep_consume(void *priv, char *line, unsigned long len)
 	if (line[0] != '+' && line[0] != '-')
 		return 0;
 	if (data->hit)
-		/*
-		 * NEEDSWORK: we should have a way to terminate the
-		 * caller early.
-		 */
-		return 0;
+		return -1;
 	/* Yuck -- line ought to be "const char *"! */
 	hold = line[len];
 	line[len] = '\0';
-- 
1.7.4.74.g639db

Re: [PATCH 1/2] xdiff-interface: allow consume function to quit early by returning non-zero

[Junio C Hamano]

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---

What is this for? No explanation?

> diff --git a/xdiff-interface.c b/xdiff-interface.c
> index 0e2c169..c5684b4 100644
> --- a/xdiff-interface.c
> +++ b/xdiff-interface.c
> @@ -56,7 +56,7 @@ int parse_hunk_header(char *line, int len,
>  	return -!!memcmp(cp, " @@", 3);
>  }
>  
> -static void consume_one(void *priv_, char *s, unsigned long size)
> +static int consume_one(void *priv_, char *s, unsigned long size)
>  {
>  	struct xdiff_emit_state *priv = priv_;
>  	char *ep;
> @@ -64,10 +64,12 @@ static void consume_one(void *priv_, char *s, unsigned long size)
>  		unsigned long this_size;
>  		ep = memchr(s, '\n', size);
>  		this_size = (ep == NULL) ? size : (ep - s + 1);
> -		priv->consume(priv->consume_callback_data, s, this_size);
> +		if (priv->consume(priv->consume_callback_data, s, this_size))
> +			return -1;
>  		size -= this_size;
>  		s += this_size;
>  	}
> +	return 0;
>  }

Returning a negative value implies that this is an error condition. Should
all "non-zero" returns from ->consume necessarily be considered error?

The same comment applies to the new "return -1" in the rest of the patch.

Re: [PATCH 1/2] xdiff-interface: allow consume function to quit early by returning non-zero

[Nguyen Thai Ngoc Duy]

2011/8/16 Junio C Hamano <gitster@pobox.com>:
> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>
>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>> ---
>
> What is this for? No explanation?

Um.. for stopping emitting diff when the consumer feels enough and
wants no more. Will update.

>> @@ -64,10 +64,12 @@ static void consume_one(void *priv_, char *s, unsigned long size)
>>               unsigned long this_size;
>>               ep = memchr(s, '\n', size);
>>               this_size = (ep == NULL) ? size : (ep - s + 1);
>> -             priv->consume(priv->consume_callback_data, s, this_size);
>> +             if (priv->consume(priv->consume_callback_data, s, this_size))
>> +                     return -1;
>>               size -= this_size;
>>               s += this_size;
>>       }
>> +     return 0;
>>  }
>
> Returning a negative value implies that this is an error condition. Should
> all "non-zero" returns from ->consume necessarily be considered error?

Perhaps negative values only, like how xdiff treats it.
-- 
Duy

Re: [PATCH 2/2] diffcore-pickaxe: terminate grepping as soon as it hits

[Nguyen Thai Ngoc Duy]

diffgrep_consume() already stops regexec() after hitting, so the value
of this series is negligible. Tay Ray Chuan seems to touch xdiff code
too and my changes are across many files. I'll drop this series.

2011/8/15 Nguyễn Thái Ngọc Duy <pclouds@gmail.com>:
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  diffcore-pickaxe.c |    6 +-----
>  1 files changed, 1 insertions(+), 5 deletions(-)
>
> diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c
> index 12811b9..e028dd5 100644
> --- a/diffcore-pickaxe.c
> +++ b/diffcore-pickaxe.c
> @@ -21,11 +21,7 @@ static int diffgrep_consume(void *priv, char *line, unsigned long len)
>        if (line[0] != '+' && line[0] != '-')
>                return 0;
>        if (data->hit)
> -               /*
> -                * NEEDSWORK: we should have a way to terminate the
> -                * caller early.
> -                */
> -               return 0;
> +               return -1;
>        /* Yuck -- line ought to be "const char *"! */
>        hold = line[len];
>        line[len] = '\0';
> --
> 1.7.4.74.g639db
>
>



-- 
Duy