{"thread":{"id":"55871","subject":"[PATCH] xdiff: use BUG(...), not xdl_bug(...)","startedAt":"2021-06-07T16:45:03Z","lastAt":"2021-06-07T19:44:06Z","messageCount":2,"participants":["Ævar Arnfjörð Bjarmason","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"426628","messageId":"patch-1.1-68bf1ba4d3-20210607T164305Z-avarab@gmail.com","threadId":"55871","inReplyTo":null,"subject":"[PATCH] xdiff: use BUG(...), not xdl_bug(...)","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-06-07T16:43:49Z","receivedAt":"2021-06-07T16:45:03Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"The xdl_bug() function was introduced in\ne8adf23d1e (xdl_change_compact(): introduce the concept of a change\ngroup, 2016-08-22), let's use our usual BUG() function instead.\n\nWe'll now have meaningful line numbers if we encounter bugs in xdiff,\nand less code duplication.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n xdiff/xdiffi.c | 22 ++++++++--------------\n 1 file changed, 8 insertions(+), 14 deletions(-)\n\ndiff --git a/xdiff/xdiffi.c b/xdiff/xdiffi.c\nindex 380eb728ed..a4542c05b6 100644\n--- a/xdiff/xdiffi.c\n+++ b/xdiff/xdiffi.c\n@@ -796,12 +796,6 @@ static int group_slide_up(xdfile_t *xdf, struct xdlgroup *g, long flags)\n \t}\n }\n \n-static void xdl_bug(const char *msg)\n-{\n-\tfprintf(stderr, \"BUG: %s\\n\", msg);\n-\texit(1);\n-}\n-\n /*\n  * Move back and forward change groups for a consistent and pretty diff output.\n  * This also helps in finding joinable change groups and reducing the diff\n@@ -841,7 +835,7 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {\n \t\t\t/* Shift the group backward as much as possible: */\n \t\t\twhile (!group_slide_up(xdf, &g, flags))\n \t\t\t\tif (group_previous(xdfo, &go))\n-\t\t\t\t\txdl_bug(\"group sync broken sliding up\");\n+\t\t\t\t\tBUG(\"group sync broken sliding up\");\n \n \t\t\t/*\n \t\t\t * This is this highest that this group can be shifted.\n@@ -857,7 +851,7 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {\n \t\t\t\tif (group_slide_down(xdf, &g, flags))\n \t\t\t\t\tbreak;\n \t\t\t\tif (group_next(xdfo, &go))\n-\t\t\t\t\txdl_bug(\"group sync broken sliding down\");\n+\t\t\t\t\tBUG(\"group sync broken sliding down\");\n \n \t\t\t\tif (go.end > go.start)\n \t\t\t\t\tend_matching_other = g.end;\n@@ -882,9 +876,9 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {\n \t\t\t */\n \t\t\twhile (go.end == go.start) {\n \t\t\t\tif (group_slide_up(xdf, &g, flags))\n-\t\t\t\t\txdl_bug(\"match disappeared\");\n+\t\t\t\t\tBUG(\"match disappeared\");\n \t\t\t\tif (group_previous(xdfo, &go))\n-\t\t\t\t\txdl_bug(\"group sync broken sliding to match\");\n+\t\t\t\t\tBUG(\"group sync broken sliding to match\");\n \t\t\t}\n \t\t} else if (flags & XDF_INDENT_HEURISTIC) {\n \t\t\t/*\n@@ -925,9 +919,9 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {\n \n \t\t\twhile (g.end > best_shift) {\n \t\t\t\tif (group_slide_up(xdf, &g, flags))\n-\t\t\t\t\txdl_bug(\"best shift unreached\");\n+\t\t\t\t\tBUG(\"best shift unreached\");\n \t\t\t\tif (group_previous(xdfo, &go))\n-\t\t\t\t\txdl_bug(\"group sync broken sliding to blank line\");\n+\t\t\t\t\tBUG(\"group sync broken sliding to blank line\");\n \t\t\t}\n \t\t}\n \n@@ -936,11 +930,11 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {\n \t\tif (group_next(xdf, &g))\n \t\t\tbreak;\n \t\tif (group_next(xdfo, &go))\n-\t\t\txdl_bug(\"group sync broken moving to next group\");\n+\t\t\tBUG(\"group sync broken moving to next group\");\n \t}\n \n \tif (!group_next(xdfo, &go))\n-\t\txdl_bug(\"group sync broken at end of file\");\n+\t\tBUG(\"group sync broken at end of file\");\n \n \treturn 0;\n }\n-- \n2.32.0.rc3.434.gd8aed1f08a7\n\n"},{"id":"426655","messageId":"YL53AylIeN+YdOTA@coredump.intra.peff.net","threadId":"55871","inReplyTo":"patch-1.1-68bf1ba4d3-20210607T164305Z-avarab@gmail.com","subject":"Re: [PATCH] xdiff: use BUG(...), not xdl_bug(...)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-06-07T19:44:03Z","receivedAt":"2021-06-07T19:44:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jun 07, 2021 at 06:43:49PM +0200, Ævar Arnfjörð Bjarmason wrote:\n\n> The xdl_bug() function was introduced in\n> e8adf23d1e (xdl_change_compact(): introduce the concept of a change\n> group, 2016-08-22), let's use our usual BUG() function instead.\n> \n> We'll now have meaningful line numbers if we encounter bugs in xdiff,\n> and less code duplication.\n\nThanks, I think this is a good direction.\n\nBack when that commit was originally done, we didn't include\ngit-compat-util.h in xdiff at all, so we couldn't rely on Git-specific\ncode (though I think BUG() did not even exist back then anyway!).\n\nThese days I think we're comfortable with the notion that there is no\nactive upstream for xdiff, so we'd not be likely to send our changes\nanywhere (and thus dependencies on our helpers are not a problem). But\nthere is one complication: libgit2 uses xdiff, too, and has pulled in\nour changes including xdl_bug().\n\nI wondered if we could make it easier on them by keeping the change in\none spot. But doing:\n\n  static void xdl_bug(const char *msg)\n  {\n\tBUG(\"%s\", msg);\n  }\n\nis not really helpful, as it loses the line numbers. I guess we could\ndo:\n\n  #ifdef BUG\n  #define xdl_bug(msg) BUG(msg)\n  #else\n  ...the current implementation\n  #endif\n\nBut really, libgit2 could just as easily define their own BUG() as they\nsee fit. Whether it is called xdl_bug() or BUG() in the call-sites is\nnot really that important.\n\nSo I think this patch is fine in its current form.\n\n-Peff\n"}]}