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 --- xdiff-interface.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'xdiff-interface.c') 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); -- cgit v1.3-6-g1900 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(-) (limited to 'xdiff-interface.c') 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.3-6-g1900 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(-) (limited to 'xdiff-interface.c') 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.3-6-g1900 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(-) (limited to 'xdiff-interface.c') 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.3-6-g1900 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(-) (limited to 'xdiff-interface.c') 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.3-6-g1900