From 611e42a5980a3a9f8bb3b1b49c1abde63c7a191e Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 2 Nov 2018 02:35:01 -0400 Subject: xdiff: provide a separate emit callback for hunks The xdiff library always emits hunk header lines to our callbacks as formatted strings like "@@ -a,b +c,d @@\n". This is convenient if we're going to output a diff, but less so if we actually need to compute using those numbers, which requires re-parsing the line. In preparation for moving away from this, let's teach xdiff a new callback function which gets the broken-out hunk information. To help callers that don't want to use this new callback, if it's NULL we'll continue to format the hunk header into a string. Note that this function renames the "outf" callback to "out_line", as well. This isn't strictly necessary, but helps in two ways: 1. Now that there are two callbacks, it's nice to use more descriptive names. 2. Many callers did not zero the emit_callback_data struct, and needed to be modified to set ecb.out_hunk to NULL. By changing the name of the existing struct member, that guarantees that any new callers from in-flight topics will break the build and be examined manually. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/merge-tree.c | 3 ++- builtin/rerere.c | 3 ++- xdiff-interface.c | 2 +- xdiff/xdiff.h | 6 +++++- xdiff/xutils.c | 21 +++++++++++++++++---- 5 files changed, 27 insertions(+), 8 deletions(-) diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c index f8023bae1e..e7771a1012 100644 --- a/builtin/merge-tree.c +++ b/builtin/merge-tree.c @@ -110,7 +110,8 @@ static void show_diff(struct merge_list *entry) xpp.flags = 0; memset(&xecfg, 0, sizeof(xecfg)); xecfg.ctxlen = 3; - ecb.outf = show_outf; + ecb.out_hunk = NULL; + ecb.out_line = show_outf; ecb.priv = NULL; src.ptr = origin(entry, &size); diff --git a/builtin/rerere.c b/builtin/rerere.c index 0bc40298c2..ea409e80cb 100644 --- a/builtin/rerere.c +++ b/builtin/rerere.c @@ -41,7 +41,8 @@ static int diff_two(const char *file1, const char *label1, xpp.flags = 0; memset(&xecfg, 0, sizeof(xecfg)); xecfg.ctxlen = 3; - ecb.outf = outf; + ecb.out_hunk = NULL; + ecb.out_line = outf; ret = xdi_diff(&minus, &plus, &xpp, &xecfg, &ecb); free(minus.ptr); diff --git a/xdiff-interface.c b/xdiff-interface.c index ec6e574e4a..88d96d78e6 100644 --- a/xdiff-interface.c +++ b/xdiff-interface.c @@ -152,7 +152,7 @@ int xdi_diff_outf(mmfile_t *mf1, mmfile_t *mf2, state.consume = fn; state.consume_callback_data = consume_callback_data; memset(&ecb, 0, sizeof(ecb)); - ecb.outf = xdiff_outf; + ecb.out_line = xdiff_outf; ecb.priv = &state; strbuf_init(&state.remainder, 0); ret = xdi_diff(mf1, mf2, xpp, xecfg, &ecb); diff --git a/xdiff/xdiff.h b/xdiff/xdiff.h index 2356da5f78..b158369020 100644 --- a/xdiff/xdiff.h +++ b/xdiff/xdiff.h @@ -86,7 +86,11 @@ typedef struct s_xpparam { typedef struct s_xdemitcb { void *priv; - int (*outf)(void *, mmbuffer_t *, int); + int (*out_hunk)(void *, + long old_begin, long old_nr, + long new_begin, long new_nr, + const char *func, long funclen); + int (*out_line)(void *, mmbuffer_t *, int); } xdemitcb_t; typedef long (*find_func_t)(const char *line, long line_len, char *buffer, long buffer_size, void *priv); diff --git a/xdiff/xutils.c b/xdiff/xutils.c index 88e5995535..963e1c58b9 100644 --- a/xdiff/xutils.c +++ b/xdiff/xutils.c @@ -54,7 +54,7 @@ int xdl_emit_diffrec(char const *rec, long size, char const *pre, long psize, mb[2].size = strlen(mb[2].ptr); i++; } - if (ecb->outf(ecb->priv, mb, i) < 0) { + if (ecb->out_line(ecb->priv, mb, i) < 0) { return -1; } @@ -344,8 +344,9 @@ int xdl_num_out(char *out, long val) { return str - out; } -int xdl_emit_hunk_hdr(long s1, long c1, long s2, long c2, - const char *func, long funclen, xdemitcb_t *ecb) { +static int xdl_format_hunk_hdr(long s1, long c1, long s2, long c2, + const char *func, long funclen, + xdemitcb_t *ecb) { int nb = 0; mmbuffer_t mb; char buf[128]; @@ -387,9 +388,21 @@ int xdl_emit_hunk_hdr(long s1, long c1, long s2, long c2, mb.ptr = buf; mb.size = nb; - if (ecb->outf(ecb->priv, &mb, 1) < 0) + if (ecb->out_line(ecb->priv, &mb, 1) < 0) return -1; + return 0; +} +int xdl_emit_hunk_hdr(long s1, long c1, long s2, long c2, + const char *func, long funclen, + xdemitcb_t *ecb) { + if (!ecb->out_hunk) + return xdl_format_hunk_hdr(s1, c1, s2, c2, func, funclen, ecb); + if (ecb->out_hunk(ecb->priv, + c1 ? s1 : s1 - 1, c1, + c2 ? s2 : s2 - 1, c2, + func, funclen) < 0) + return -1; return 0; } -- cgit v1.2.3 From 9346d6d14dddc7989ba879839d58f6c2426cffbb Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 2 Nov 2018 02:35:45 -0400 Subject: xdiff-interface: provide a separate consume callback for hunks The previous commit taught xdiff to optionally provide the hunk header data to a specialized callback. But most users of xdiff actually use our more convenient xdi_diff_outf() helper, which ensures that our callbacks are always fed whole lines. Let's plumb the special hunk-callback through this interface, too. It will follow the same rule as xdiff when the hunk callback is NULL (i.e., continue to pass a stringified hunk header to the line callback). Since we add NULL to each caller, there should be no behavior change yet. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- combine-diff.c | 4 ++-- diff.c | 20 ++++++++++---------- diffcore-pickaxe.c | 2 +- range-diff.c | 2 +- xdiff-interface.c | 30 ++++++++++++++++++++++++++---- xdiff-interface.h | 10 ++++++++-- 6 files changed, 48 insertions(+), 20 deletions(-) diff --git a/combine-diff.c b/combine-diff.c index de7695e728..195054b87a 100644 --- a/combine-diff.c +++ b/combine-diff.c @@ -419,8 +419,8 @@ static void combine_diff(const struct object_id *parent, unsigned int mode, state.num_parent = num_parent; state.n = n; - if (xdi_diff_outf(&parent_file, result_file, consume_line, &state, - &xpp, &xecfg)) + if (xdi_diff_outf(&parent_file, result_file, NULL, consume_line, + &state, &xpp, &xecfg)) die("unable to generate combined diff for %s", oid_to_hex(parent)); free(parent_file.ptr); diff --git a/diff.c b/diff.c index 145cfbae59..f9b9adc545 100644 --- a/diff.c +++ b/diff.c @@ -2045,8 +2045,8 @@ static void diff_words_show(struct diff_words_data *diff_words) xpp.flags = 0; /* as only the hunk header will be parsed, we need a 0-context */ xecfg.ctxlen = 0; - if (xdi_diff_outf(&minus, &plus, fn_out_diff_words_aux, diff_words, - &xpp, &xecfg)) + if (xdi_diff_outf(&minus, &plus, NULL, fn_out_diff_words_aux, + diff_words, &xpp, &xecfg)) die("unable to generate word diff"); free(minus.ptr); free(plus.ptr); @@ -3495,8 +3495,8 @@ static void builtin_diff(const char *name_a, xecfg.ctxlen = strtoul(v, NULL, 10); if (o->word_diff) init_diff_words_data(&ecbdata, o, one, two); - if (xdi_diff_outf(&mf1, &mf2, fn_out_consume, &ecbdata, - &xpp, &xecfg)) + if (xdi_diff_outf(&mf1, &mf2, NULL, fn_out_consume, + &ecbdata, &xpp, &xecfg)) die("unable to generate diff for %s", one->path); if (o->word_diff) free_diff_words_data(&ecbdata); @@ -3604,8 +3604,8 @@ static void builtin_diffstat(const char *name_a, const char *name_b, xpp.anchors_nr = o->anchors_nr; xecfg.ctxlen = o->context; xecfg.interhunkctxlen = o->interhunkcontext; - if (xdi_diff_outf(&mf1, &mf2, diffstat_consume, diffstat, - &xpp, &xecfg)) + if (xdi_diff_outf(&mf1, &mf2, NULL, diffstat_consume, + diffstat, &xpp, &xecfg)) die("unable to generate diffstat for %s", one->path); } @@ -3652,8 +3652,8 @@ static void builtin_checkdiff(const char *name_a, const char *name_b, memset(&xecfg, 0, sizeof(xecfg)); xecfg.ctxlen = 1; /* at least one context line */ xpp.flags = 0; - if (xdi_diff_outf(&mf1, &mf2, checkdiff_consume, &data, - &xpp, &xecfg)) + if (xdi_diff_outf(&mf1, &mf2, NULL, checkdiff_consume, + &data, &xpp, &xecfg)) die("unable to generate checkdiff for %s", one->path); if (data.ws_rule & WS_BLANK_AT_EOF) { @@ -5712,8 +5712,8 @@ static int diff_get_patch_id(struct diff_options *options, struct object_id *oid xpp.flags = 0; xecfg.ctxlen = 3; xecfg.flags = 0; - if (xdi_diff_outf(&mf1, &mf2, patch_id_consume, &data, - &xpp, &xecfg)) + if (xdi_diff_outf(&mf1, &mf2, NULL, patch_id_consume, + &data, &xpp, &xecfg)) return error("unable to generate patch-id diff for %s", p->one->path); } diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c index 800a899c86..7609bb4fe1 100644 --- a/diffcore-pickaxe.c +++ b/diffcore-pickaxe.c @@ -62,7 +62,7 @@ static int diff_grep(mmfile_t *one, mmfile_t *two, ecbdata.hit = 0; xecfg.ctxlen = o->context; xecfg.interhunkctxlen = o->interhunkcontext; - if (xdi_diff_outf(one, two, diffgrep_consume, &ecbdata, &xpp, &xecfg)) + if (xdi_diff_outf(one, two, NULL, diffgrep_consume, &ecbdata, &xpp, &xecfg)) return 0; return ecbdata.hit; } diff --git a/range-diff.c b/range-diff.c index b6b9abac26..0f4ce140dc 100644 --- a/range-diff.c +++ b/range-diff.c @@ -190,7 +190,7 @@ static int diffsize(const char *a, const char *b) mf2.size = strlen(b); cfg.ctxlen = 3; - if (!xdi_diff_outf(&mf1, &mf2, diffsize_consume, &count, &pp, &cfg)) + if (!xdi_diff_outf(&mf1, &mf2, NULL, diffsize_consume, &count, &pp, &cfg)) return count; error(_("failed to generate diff")); diff --git a/xdiff-interface.c b/xdiff-interface.c index 88d96d78e6..eb9c05a1e3 100644 --- a/xdiff-interface.c +++ b/xdiff-interface.c @@ -9,7 +9,8 @@ #include "xdiff/xutils.h" struct xdiff_emit_state { - xdiff_emit_consume_fn consume; + xdiff_emit_hunk_fn hunk_fn; + xdiff_emit_line_fn line_fn; void *consume_callback_data; struct strbuf remainder; }; @@ -59,6 +60,22 @@ int parse_hunk_header(char *line, int len, return -!!memcmp(cp, " @@", 3); } +static int xdiff_out_hunk(void *priv_, + long old_begin, long old_nr, + long new_begin, long new_nr, + const char *func, long funclen) +{ + struct xdiff_emit_state *priv = priv_; + + if (priv->remainder.len) + BUG("xdiff emitted hunk in the middle of a line"); + + priv->hunk_fn(priv->consume_callback_data, + old_begin, old_nr, new_begin, new_nr, + func, funclen); + return 0; +} + static void consume_one(void *priv_, char *s, unsigned long size) { struct xdiff_emit_state *priv = priv_; @@ -67,7 +84,7 @@ 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); + priv->line_fn(priv->consume_callback_data, s, this_size); size -= this_size; s += this_size; } @@ -141,7 +158,9 @@ int xdi_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, xdemitconf_t co } int xdi_diff_outf(mmfile_t *mf1, mmfile_t *mf2, - xdiff_emit_consume_fn fn, void *consume_callback_data, + xdiff_emit_hunk_fn hunk_fn, + xdiff_emit_line_fn line_fn, + void *consume_callback_data, xpparam_t const *xpp, xdemitconf_t const *xecfg) { int ret; @@ -149,9 +168,12 @@ int xdi_diff_outf(mmfile_t *mf1, mmfile_t *mf2, xdemitcb_t ecb; memset(&state, 0, sizeof(state)); - state.consume = fn; + state.hunk_fn = hunk_fn; + state.line_fn = line_fn; state.consume_callback_data = consume_callback_data; memset(&ecb, 0, sizeof(ecb)); + if (hunk_fn) + ecb.out_hunk = xdiff_out_hunk; ecb.out_line = xdiff_outf; ecb.priv = &state; strbuf_init(&state.remainder, 0); diff --git a/xdiff-interface.h b/xdiff-interface.h index 135fc05d72..2dbe2feb19 100644 --- a/xdiff-interface.h +++ b/xdiff-interface.h @@ -11,11 +11,17 @@ */ #define MAX_XDIFF_SIZE (1024UL * 1024 * 1023) -typedef void (*xdiff_emit_consume_fn)(void *, char *, unsigned long); +typedef void (*xdiff_emit_line_fn)(void *, char *, unsigned long); +typedef void (*xdiff_emit_hunk_fn)(void *data, + long old_begin, long old_nr, + long new_begin, long new_nr, + const char *func, long funclen); int xdi_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, xdemitconf_t const *xecfg, xdemitcb_t *ecb); int xdi_diff_outf(mmfile_t *mf1, mmfile_t *mf2, - xdiff_emit_consume_fn fn, void *consume_callback_data, + xdiff_emit_hunk_fn hunk_fn, + xdiff_emit_line_fn line_fn, + void *consume_callback_data, xpparam_t const *xpp, xdemitconf_t const *xecfg); int parse_hunk_header(char *line, int len, int *ob, int *on, -- cgit v1.2.3 From 3b40a090fd4e441e88897dfa96f50039952ed45b Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 2 Nov 2018 02:36:06 -0400 Subject: diff: avoid generating unused hunk header lines Some callers of xdi_diff_outf() do not look at the generated hunk header lines at all. By plugging in a no-op hunk callback, this tells xdiff not to even bother formatting them. This patch introduces a stock no-op callback and uses it with a few callers whose line callbacks explicitly ignore hunk headers (because they look only for +/- lines). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- diff.c | 4 ++-- diffcore-pickaxe.c | 3 ++- xdiff-interface.c | 6 ++++++ xdiff-interface.h | 8 ++++++++ 4 files changed, 18 insertions(+), 3 deletions(-) diff --git a/diff.c b/diff.c index f9b9adc545..d3e7262310 100644 --- a/diff.c +++ b/diff.c @@ -3604,8 +3604,8 @@ static void builtin_diffstat(const char *name_a, const char *name_b, xpp.anchors_nr = o->anchors_nr; xecfg.ctxlen = o->context; xecfg.interhunkctxlen = o->interhunkcontext; - if (xdi_diff_outf(&mf1, &mf2, NULL, diffstat_consume, - diffstat, &xpp, &xecfg)) + if (xdi_diff_outf(&mf1, &mf2, discard_hunk_line, + diffstat_consume, diffstat, &xpp, &xecfg)) die("unable to generate diffstat for %s", one->path); } diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c index 7609bb4fe1..25e12148e4 100644 --- a/diffcore-pickaxe.c +++ b/diffcore-pickaxe.c @@ -62,7 +62,8 @@ static int diff_grep(mmfile_t *one, mmfile_t *two, ecbdata.hit = 0; xecfg.ctxlen = o->context; xecfg.interhunkctxlen = o->interhunkcontext; - if (xdi_diff_outf(one, two, NULL, diffgrep_consume, &ecbdata, &xpp, &xecfg)) + if (xdi_diff_outf(one, two, discard_hunk_line, diffgrep_consume, + &ecbdata, &xpp, &xecfg)) return 0; return ecbdata.hit; } diff --git a/xdiff-interface.c b/xdiff-interface.c index eb9c05a1e3..b99a57825f 100644 --- a/xdiff-interface.c +++ b/xdiff-interface.c @@ -157,6 +157,12 @@ int xdi_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, xdemitconf_t co return xdl_diff(&a, &b, xpp, xecfg, xecb); } +void discard_hunk_line(void *priv, + long ob, long on, long nb, long nn, + const char *func, long funclen) +{ +} + int xdi_diff_outf(mmfile_t *mf1, mmfile_t *mf2, xdiff_emit_hunk_fn hunk_fn, xdiff_emit_line_fn line_fn, diff --git a/xdiff-interface.h b/xdiff-interface.h index 2dbe2feb19..8af245eed9 100644 --- a/xdiff-interface.h +++ b/xdiff-interface.h @@ -35,6 +35,14 @@ extern void xdiff_clear_find_func(xdemitconf_t *xecfg); extern int git_xmerge_config(const char *var, const char *value, void *cb); extern int git_xmerge_style; +/* + * Can be used as a no-op hunk_fn for xdi_diff_outf(), since a NULL + * one just sends the hunk line to the line_fn callback). + */ +void discard_hunk_line(void *priv, + long ob, long on, long nb, long nn, + const char *func, long funclen); + /* * Compare the strings l1 with l2 which are of size s1 and s2 respectively. * Returns 1 if the strings are deemed equal, 0 otherwise. -- cgit v1.2.3 From b135739125e9dc49fafc42d4a8a1956c46329ff1 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 2 Nov 2018 02:36:36 -0400 Subject: diff: discard hunk headers for patch-ids earlier We do not include hunk header lines when computing patch-ids, since the line numbers would create false negatives. Rather than detect and skip them in our line callback, we can simply tell xdiff to avoid generating them. This is similar to the previous commit, but split out because it actually requires modifying the matching line callback. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- diff.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/diff.c b/diff.c index d3e7262310..ab55b0466e 100644 --- a/diff.c +++ b/diff.c @@ -5607,10 +5607,6 @@ static void 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 (starts_with(line, "@@ -")) - return; - new_len = remove_space(line, len); git_SHA1_Update(data->ctx, line, new_len); @@ -5712,8 +5708,8 @@ static int diff_get_patch_id(struct diff_options *options, struct object_id *oid xpp.flags = 0; xecfg.ctxlen = 3; xecfg.flags = 0; - if (xdi_diff_outf(&mf1, &mf2, NULL, patch_id_consume, - &data, &xpp, &xecfg)) + if (xdi_diff_outf(&mf1, &mf2, discard_hunk_line, + patch_id_consume, &data, &xpp, &xecfg)) return error("unable to generate patch-id diff for %s", p->one->path); } -- cgit v1.2.3 From 7c61e25fbf1a4a65208be1197940a383f220a1b7 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 2 Nov 2018 02:37:18 -0400 Subject: diff: use hunk callback for word-diff Our word-diff does not look at the -/+ lines generated by xdiff at all (because they are not real lines to show the user, but just the tokenized words split into lines). Instead we use the line numbers from the hunk headers to index our own data structure. As a result, our xdi_diff_outf() callback throws away all lines except hunk headers. We can instead use a hunk callback, which has two benefits: 1. We don't have to re-parse the generated hunk header line, but can use the passed parameters directly. 2. By setting our line callback to NULL, we can tell xdiff-interface that it does not even need to bother generating the other lines, saving a small amount of work. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- diff.c | 12 +++++------- xdiff-interface.c | 3 +++ 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/diff.c b/diff.c index ab55b0466e..be312bc20d 100644 --- a/diff.c +++ b/diff.c @@ -1883,19 +1883,17 @@ 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 void fn_out_diff_words_aux(void *priv, + long minus_first, long minus_len, + long plus_first, long plus_len, + const char *func, long funclen) { struct diff_words_data *diff_words = priv; struct diff_words_style *style = diff_words->style; - int minus_first, minus_len, plus_first, plus_len; const char *minus_begin, *minus_end, *plus_begin, *plus_end; struct diff_options *opt = diff_words->opt; const char *line_prefix; - if (line[0] != '@' || parse_hunk_header(line, len, - &minus_first, &minus_len, &plus_first, &plus_len)) - return; - assert(opt); line_prefix = diff_line_prefix(opt); @@ -2045,7 +2043,7 @@ static void diff_words_show(struct diff_words_data *diff_words) xpp.flags = 0; /* as only the hunk header will be parsed, we need a 0-context */ xecfg.ctxlen = 0; - if (xdi_diff_outf(&minus, &plus, NULL, fn_out_diff_words_aux, + if (xdi_diff_outf(&minus, &plus, fn_out_diff_words_aux, NULL, diff_words, &xpp, &xecfg)) die("unable to generate word diff"); free(minus.ptr); diff --git a/xdiff-interface.c b/xdiff-interface.c index b99a57825f..a23adb3c45 100644 --- a/xdiff-interface.c +++ b/xdiff-interface.c @@ -95,6 +95,9 @@ static int xdiff_outf(void *priv_, mmbuffer_t *mb, int nbuf) struct xdiff_emit_state *priv = priv_; int i; + if (!priv->line_fn) + return 0; + for (i = 0; i < nbuf; i++) { if (mb[i].ptr[mb[i].size-1] != '\n') { /* Incomplete line */ -- cgit v1.2.3 From 0074c9110db6b8cddc9950f94f69d143379b25ed Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 2 Nov 2018 02:38:20 -0400 Subject: combine-diff: use an xdiff hunk callback A combined diff has to line up the hunks for all of the individual pairwise diffs, and thus needs to know their line numbers and sizes. We get that now by parsing the hunk header line that xdiff generates. However, now that xdiff supports a hunk callback, we can just use the values directly. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- combine-diff.c | 67 +++++++++++++++++++++++++++++++--------------------------- 1 file changed, 36 insertions(+), 31 deletions(-) diff --git a/combine-diff.c b/combine-diff.c index 195054b87a..e585683ea9 100644 --- a/combine-diff.c +++ b/combine-diff.c @@ -344,38 +344,43 @@ struct combine_diff_state { struct sline *lost_bucket; }; -static void consume_line(void *state_, char *line, unsigned long len) +static void consume_hunk(void *state_, + long ob, long on, + long nb, long nn, + const char *funcline, long funclen) { 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; - state->lno = state->nb; - if (state->nn == 0) { - /* @@ -X,Y +N,0 @@ removed Y lines - * that would have come *after* line N - * in the result. Our lost buckets hang - * to the line after the removed lines, - * - * Note that this is correct even when N == 0, - * in which case the hunk removes the first - * line in the file. - */ - state->lost_bucket = &state->sline[state->nb]; - if (!state->nb) - state->nb = 1; - } else { - state->lost_bucket = &state->sline[state->nb-1]; - } - if (!state->sline[state->nb-1].p_lno) - state->sline[state->nb-1].p_lno = - xcalloc(state->num_parent, - sizeof(unsigned long)); - state->sline[state->nb-1].p_lno[state->n] = state->ob; - return; + + state->ob = ob; + state->on = on; + state->nb = nb; + state->nn = nn; + state->lno = state->nb; + if (state->nn == 0) { + /* @@ -X,Y +N,0 @@ removed Y lines + * that would have come *after* line N + * in the result. Our lost buckets hang + * to the line after the removed lines, + * + * Note that this is correct even when N == 0, + * in which case the hunk removes the first + * line in the file. + */ + state->lost_bucket = &state->sline[state->nb]; + if (!state->nb) + state->nb = 1; + } else { + state->lost_bucket = &state->sline[state->nb-1]; } + if (!state->sline[state->nb-1].p_lno) + state->sline[state->nb-1].p_lno = + xcalloc(state->num_parent, sizeof(unsigned long)); + state->sline[state->nb-1].p_lno[state->n] = state->ob; +} + +static void consume_line(void *state_, char *line, unsigned long len) +{ + struct combine_diff_state *state = state_; if (!state->lost_bucket) return; /* not in any hunk yet */ switch (line[0]) { @@ -419,8 +424,8 @@ static void combine_diff(const struct object_id *parent, unsigned int mode, state.num_parent = num_parent; state.n = n; - if (xdi_diff_outf(&parent_file, result_file, NULL, consume_line, - &state, &xpp, &xecfg)) + if (xdi_diff_outf(&parent_file, result_file, consume_hunk, + consume_line, &state, &xpp, &xecfg)) die("unable to generate combined diff for %s", oid_to_hex(parent)); free(parent_file.ptr); -- cgit v1.2.3 From 75ab76306cb97b223b29e9460a9589cfd099213e Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 2 Nov 2018 02:39:03 -0400 Subject: diff: convert --check to use a hunk callback The "diff --check" code needs to know the line number on which each hunk starts in order to generate its output. We get that now by parsing the hunk header line generated by xdiff, but it's much simpler to just pass it directly using a hunk callback. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- diff.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/diff.c b/diff.c index be312bc20d..6a34bbeeeb 100644 --- a/diff.c +++ b/diff.c @@ -3101,6 +3101,15 @@ static int is_conflict_marker(const char *line, int marker_size, unsigned long l return 1; } +static void checkdiff_consume_hunk(void *priv, + long ob, long on, long nb, long nn, + const char *func, long funclen) + +{ + struct checkdiff_t *data = priv; + data->lineno = nb - 1; +} + static void checkdiff_consume(void *priv, char *line, unsigned long len) { struct checkdiff_t *data = priv; @@ -3136,12 +3145,6 @@ static void checkdiff_consume(void *priv, char *line, unsigned long len) data->o->file, set, reset, ws); } else if (line[0] == ' ') { data->lineno++; - } else if (line[0] == '@') { - char *plus = strchr(line, '+'); - if (plus) - data->lineno = strtol(plus, NULL, 10) - 1; - else - die("invalid diff"); } } @@ -3650,8 +3653,9 @@ static void builtin_checkdiff(const char *name_a, const char *name_b, memset(&xecfg, 0, sizeof(xecfg)); xecfg.ctxlen = 1; /* at least one context line */ xpp.flags = 0; - if (xdi_diff_outf(&mf1, &mf2, NULL, checkdiff_consume, - &data, &xpp, &xecfg)) + if (xdi_diff_outf(&mf1, &mf2, checkdiff_consume_hunk, + checkdiff_consume, &data, + &xpp, &xecfg)) die("unable to generate checkdiff for %s", one->path); if (data.ws_rule & WS_BLANK_AT_EOF) { -- cgit v1.2.3 From d2eb80935a4e93cd775b5e8dc3f07fa1cd21d330 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 2 Nov 2018 02:39:40 -0400 Subject: range-diff: use a hunk callback When we count the lines in a diff, we don't actually care about the contents of each line. By using a hunk callback, we tell xdiff that it does not need to even bother generating a hunk header line, saving a small amount of work. Arguably we could even ignore the hunk headers completely, since we're just computing a cost function between patches. But doing it this way maintains the exact same behavior before and after. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- range-diff.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/range-diff.c b/range-diff.c index 0f4ce140dc..2ef372bbac 100644 --- a/range-diff.c +++ b/range-diff.c @@ -177,6 +177,12 @@ static void diffsize_consume(void *data, char *line, unsigned long len) (*(int *)data)++; } +static void diffsize_hunk(void *data, long ob, long on, long nb, long nn, + const char *funcline, long funclen) +{ + diffsize_consume(data, NULL, 0); +} + static int diffsize(const char *a, const char *b) { xpparam_t pp = { 0 }; @@ -190,7 +196,9 @@ static int diffsize(const char *a, const char *b) mf2.size = strlen(b); cfg.ctxlen = 3; - if (!xdi_diff_outf(&mf1, &mf2, NULL, diffsize_consume, &count, &pp, &cfg)) + if (!xdi_diff_outf(&mf1, &mf2, + diffsize_hunk, diffsize_consume, &count, + &pp, &cfg)) return count; error(_("failed to generate diff")); -- cgit v1.2.3 From 5eade0746e1daf659a9559d804068f9f31614625 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 2 Nov 2018 02:40:13 -0400 Subject: xdiff-interface: drop parse_hunk_header() This function was used only for parsing the hunk headers generated by xdiff. Now that we can use hunk callbacks to get that information directly, it has outlived its usefulness. Note to anyone who wants to resurrect it: the "len" parameter was totally unused, meaning that the function could read past the end of the "line" array. In practice this never happened, because we only used it to parse xdiff's generated header lines. But it would be dangerous to use it for other cases without fixing this defect. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- xdiff-interface.c | 45 --------------------------------------------- xdiff-interface.h | 3 --- 2 files changed, 48 deletions(-) diff --git a/xdiff-interface.c b/xdiff-interface.c index a23adb3c45..de9c524d2f 100644 --- a/xdiff-interface.c +++ b/xdiff-interface.c @@ -15,51 +15,6 @@ struct xdiff_emit_state { struct strbuf remainder; }; -static int parse_num(char **cp_p, int *num_p) -{ - char *cp = *cp_p; - int num = 0; - - while ('0' <= *cp && *cp <= '9') - num = num * 10 + *cp++ - '0'; - if (!(cp - *cp_p)) - return -1; - *cp_p = cp; - *num_p = num; - return 0; -} - -int parse_hunk_header(char *line, int len, - int *ob, int *on, - int *nb, int *nn) -{ - char *cp; - cp = line + 4; - if (parse_num(&cp, ob)) { - bad_line: - return error("malformed diff output: %s", line); - } - if (*cp == ',') { - cp++; - if (parse_num(&cp, on)) - goto bad_line; - } - else - *on = 1; - if (*cp++ != ' ' || *cp++ != '+') - goto bad_line; - if (parse_num(&cp, nb)) - goto bad_line; - if (*cp == ',') { - cp++; - if (parse_num(&cp, nn)) - goto bad_line; - } - else - *nn = 1; - return -!!memcmp(cp, " @@", 3); -} - static int xdiff_out_hunk(void *priv_, long old_begin, long old_nr, long new_begin, long new_nr, diff --git a/xdiff-interface.h b/xdiff-interface.h index 8af245eed9..2d41fffd4c 100644 --- a/xdiff-interface.h +++ b/xdiff-interface.h @@ -23,9 +23,6 @@ int xdi_diff_outf(mmfile_t *mf1, mmfile_t *mf2, xdiff_emit_line_fn line_fn, void *consume_callback_data, xpparam_t const *xpp, xdemitconf_t const *xecfg); -int parse_hunk_header(char *line, int len, - int *ob, int *on, - int *nb, int *nn); int read_mmfile(mmfile_t *ptr, const char *filename); void read_mmblob(mmfile_t *ptr, const struct object_id *oid); int buffer_is_binary(const char *ptr, unsigned long size); -- cgit v1.2.3