Skip to content

Commit 9e20442

Browse files
avargitster
authored andcommitted
xdiff-interface: allow early return from xdiff_emit_line_fn
Finish the change started in the preceding commit and allow an early return from "xdiff_emit_line_fn" callbacks, this will allows diffcore-pickaxe.c to save itself redundant work. Our xdiff interface also had the limitation of not being able to abort early since the beginning, see d9ea73e (combine-diff: refactor built-in xdiff interface., 2006-04-05). Although at that time "xdiff_emit_line_fn" was called "xdiff_emit_consume_fn", and "xdiff_emit_hunk_fn" didn't exist yet. There was some work in this area of xdiff-interface.[ch] recently with 3b40a09 (diff: avoid generating unused hunk header lines, 2018-11-02) and 7c61e25 (diff: use hunk callback for word-diff, 2018-11-02). In combination those two changes allow us to not do any work on the hunks and diff at all, but didn't change the status quo with regards to consumers that e.g. want the diff lines, but might want to abort early. Whereas now we can abort e.g. on the first "-line" of a 1000 line diff if that's all we needed. This interface is rather scary as noted in the comment to xdiff-interface.h being added here, as noted there a future change could add more exit codes, and hack xdl_emit_diff() and friends to ignore or skip things more selectively as a result. I did not see an inherent reason for why xdl_emit_{diffrec,record}() could not be changed to ferry the "xdiff_emit_line_fn" error code upwards instead of returning -1 on all "ret < 0". But doing so would require corresponding changes in xdl_emit_diff(), xdl_diff(). I didn't see any issue with narrowly doing that to accomplish what I needed here, but it would leave xdiff's own return values in an inconsistent state. Instead I've left it at returning a more conventional (for git's own codebase) 1 for an early return, and translating it (or rather, all non-zero) to -1 for xdiff's consumption. The reason for most of the "stop" complexity in xdiff_outf() is because we want to be able to abort early, but do so in a way that doesn't skip the appropriate strbuf_reset() invocations. Signed-off-by: Ævar Arnfjörð Bjarmason <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent a8d5eb6 commit 9e20442

File tree

2 files changed

+31
-4
lines changed

2 files changed

+31
-4
lines changed

xdiff-interface.c

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,12 @@ static int consume_one(void *priv_, char *s, unsigned long size)
3737
char *ep;
3838
while (size) {
3939
unsigned long this_size;
40+
int ret;
4041
ep = memchr(s, '\n', size);
4142
this_size = (ep == NULL) ? size : (ep - s + 1);
42-
priv->line_fn(priv->consume_callback_data, s, this_size);
43+
ret = priv->line_fn(priv->consume_callback_data, s, this_size);
44+
if (ret)
45+
return ret;
4346
size -= this_size;
4447
s += this_size;
4548
}
@@ -50,11 +53,14 @@ static int xdiff_outf(void *priv_, mmbuffer_t *mb, int nbuf)
5053
{
5154
struct xdiff_emit_state *priv = priv_;
5255
int i;
56+
int stop = 0;
5357

5458
if (!priv->line_fn)
5559
return 0;
5660

5761
for (i = 0; i < nbuf; i++) {
62+
if (stop)
63+
return 1;
5864
if (mb[i].ptr[mb[i].size-1] != '\n') {
5965
/* Incomplete line */
6066
strbuf_add(&priv->remainder, mb[i].ptr, mb[i].size);
@@ -63,17 +69,21 @@ static int xdiff_outf(void *priv_, mmbuffer_t *mb, int nbuf)
6369

6470
/* we have a complete line */
6571
if (!priv->remainder.len) {
66-
consume_one(priv, mb[i].ptr, mb[i].size);
72+
stop = consume_one(priv, mb[i].ptr, mb[i].size);
6773
continue;
6874
}
6975
strbuf_add(&priv->remainder, mb[i].ptr, mb[i].size);
70-
consume_one(priv, priv->remainder.buf, priv->remainder.len);
76+
stop = consume_one(priv, priv->remainder.buf, priv->remainder.len);
7177
strbuf_reset(&priv->remainder);
7278
}
79+
if (stop)
80+
return -1;
7381
if (priv->remainder.len) {
74-
consume_one(priv, priv->remainder.buf, priv->remainder.len);
82+
stop = consume_one(priv, priv->remainder.buf, priv->remainder.len);
7583
strbuf_reset(&priv->remainder);
7684
}
85+
if (stop)
86+
return -1;
7787
return 0;
7888
}
7989

xdiff-interface.h

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,23 @@
1111
*/
1212
#define MAX_XDIFF_SIZE (1024UL * 1024 * 1023)
1313

14+
/**
15+
* The `xdiff_emit_line_fn` function can return 1 to abort early, or 0
16+
* to continue processing. Note that doing so is an all-or-nothing
17+
* affair, as returning 1 will return all the way to the top-level,
18+
* e.g. the xdi_diff_outf() call to generate the diff.
19+
*
20+
* Thus returning 1 means you won't be getting any more diff lines. If
21+
* you need something in-between those two options you'll to use
22+
* `xdl_emit_hunk_consume_func_t` and implement your own version of
23+
* xdl_emit_diff().
24+
*
25+
* We may extend the interface in the future to understand other more
26+
* granular return values. While you should return 1 to exit early,
27+
* doing so will currently make your early return indistinguishable
28+
* from an error internal to xdiff, xdiff itself will see that
29+
* non-zero return and translate it to -1.
30+
*/
1431
typedef int (*xdiff_emit_line_fn)(void *, char *, unsigned long);
1532
typedef void (*xdiff_emit_hunk_fn)(void *data,
1633
long old_begin, long old_nr,

0 commit comments

Comments
 (0)