[PATCH v3 18/22] xdiff-interface: allow early return from xdiff_emit_line_fn
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Apr 12, 2021, 17:15 UTC
- Message-ID
- <patch-18.22-76d667f152f-20210412T170457Z-avarab@gmail.com>
- In-Reply-To
- <cover-00.22-00000000000-20210412T170457Z-avarab@gmail.com>
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 d9ea73e0564 (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 3b40a090fd4 (diff: avoid generating unused hunk header lines, 2018-11-02) and 7c61e25fbf1 (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 <avarab@gmail.com> --- xdiff-interface.c | 18 ++++++++++++++---- xdiff-interface.h | 17 +++++++++++++++++ 2 files changed, 31 insertions(+), 4 deletions(-)
diff --git a/xdiff-interface.c b/xdiff-interface.c index 5d8c8c67dc2..50c0ef759dd 100644 --- a/xdiff-interface.c +++ b/xdiff-interface.c @@ -37,9 +37,12 @@ static int consume_one(void *priv_, char *s, unsigned long size) char *ep; while (size) { unsigned long this_size; + int ret; ep = memchr(s, '\n', size); this_size = (ep == NULL) ? size : (ep - s + 1); - priv->line_fn(priv->consume_callback_data, s, this_size); + ret = priv->line_fn(priv->consume_callback_data, s, this_size); + if (ret) + return ret; size -= this_size; s += this_size; } @@ -50,11 +53,14 @@ static int xdiff_outf(void *priv_, mmbuffer_t *mb, int nbuf) { struct xdiff_emit_state *priv = priv_; int i; + int stop = 0; if (!priv->line_fn) return 0; for (i = 0; i < nbuf; i++) { + if (stop) + return 1; if (mb[i].ptr[mb[i].size-1] != '\n') { /* Incomplete line */ 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) /* we have a complete line */ if (!priv->remainder.len) { - consume_one(priv, mb[i].ptr, mb[i].size); + stop = consume_one(priv, mb[i].ptr, mb[i].size); continue; } strbuf_add(&priv->remainder, mb[i].ptr, mb[i].size); - consume_one(priv, priv->remainder.buf, priv->remainder.len); + stop = consume_one(priv, priv->remainder.buf, priv->remainder.len); strbuf_reset(&priv->remainder); } + if (stop) + return -1; if (priv->remainder.len) { - consume_one(priv, priv->remainder.buf, priv->remainder.len); + stop = consume_one(priv, priv->remainder.buf, priv->remainder.len); strbuf_reset(&priv->remainder); } + if (stop) + return -1; return 0; } diff --git a/xdiff-interface.h b/xdiff-interface.h index 0198f9632f5..7d1724abb64 100644 --- a/xdiff-interface.h +++ b/xdiff-interface.h @@ -11,6 +11,23 @@ */ #define MAX_XDIFF_SIZE (1024UL * 1024 * 1023) +/** + * The `xdiff_emit_line_fn` function can return 1 to abort early, or 0 + * to continue processing. Note that doing so is an all-or-nothing + * affair, as returning 1 will return all the way to the top-level, + * e.g. the xdi_diff_outf() call to generate the diff. + * + * Thus returning 1 means you won't be getting any more diff lines. If + * you need something in-between those two options you'll to use + * `xdl_emit_hunk_consume_func_t` and implement your own version of + * xdl_emit_diff(). + * + * We may extend the interface in the future to understand other more + * granular return values. While you should return 1 to exit early, + * doing so will currently make your early return indistinguishable + * from an error internal to xdiff, xdiff itself will see that + * non-zero return and translate it to -1. + */ typedef int (*xdiff_emit_line_fn)(void *, char *, unsigned long); typedef void (*xdiff_emit_hunk_fn)(void *data, long old_begin, long old_nr,
-- 2.31.1.639.g3d04783866f