{"thread":{"id":"57433","subject":"[PATCH v2 0/1] xdiff: provide indirection to git functions","startedAt":"2022-02-17T22:52:31Z","lastAt":"2022-02-25T19:03:33Z","messageCount":8,"participants":["Edward Thomson","Phillip Wood","Johannes Schindelin","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":1},"messages":[{"id":"448744","messageId":"20220217225218.GA7@edef91d97c94","threadId":"57433","inReplyTo":null,"subject":"[PATCH v2 0/1] xdiff: provide indirection to git functions","fromName":"Edward Thomson","fromEmail":"ethomson@edwardthomson.com","sentAt":"2022-02-17T22:52:18Z","receivedAt":"2022-02-17T22:52:31Z","isPatch":true,"sender":{"key":"ethomson@edwardthomson.com","avatar":"https://avatars.githubusercontent.com/u/1130014?v=4"},"body":"Hello (again) from libgit2; this is a v2 of changes to xdiff to allow\nus to work together more easily.  As discussed in the previous patch\n(https://lore.kernel.org/git/20220209012951.GA7@abe733c6e288/) this\nadds a simple abstraction layer in `git-xdiff.h`.\n\nOther xdiff users, like libgit2, can specify their own compatibility\nfunctions in this header file.\n\nCheers-\n-ed\n\nEdward Thomson (1):\n  xdiff: provide indirection to git functions\n\n xdiff/git-xdiff.h | 16 ++++++++++++++++\n xdiff/xdiff.h     |  8 +++-----\n xdiff/xdiffi.c    | 20 ++++++++++----------\n xdiff/xinclude.h  |  2 +-\n xdiff/xmerge.c    |  4 ++--\n 5 files changed, 32 insertions(+), 18 deletions(-)\n create mode 100644 xdiff/git-xdiff.h\n\n--\n2.35.1\n"},{"id":"448745","messageId":"20220217225408.GB7@edef91d97c94","threadId":"57433","inReplyTo":"20220217225218.GA7@edef91d97c94","subject":"[PATCH v2 1/1] xdiff: provide indirection to git functions","fromName":"Edward Thomson","fromEmail":"ethomson@edwardthomson.com","sentAt":"2022-02-17T22:54:08Z","receivedAt":"2022-02-17T22:54:17Z","isPatch":true,"sender":{"key":"ethomson@edwardthomson.com","avatar":"https://avatars.githubusercontent.com/u/1130014?v=4"},"body":"Provide an indirection layer into the git-specific functionality and\nutilities in `git-xdiff.h`, prefixing those types and functions with\n`xdl_` (and `XDL_` for macros).  This allows other projects that use\ngit's xdiff implementation to keep up-to-date; they can now take all the\nfiles _except_ `git-xdiff.h`, which they have customized for their own\nenvironment.\n\nSigned-off-by: Edward Thomson <ethomson@edwardthomson.com>\n---\n xdiff/git-xdiff.h | 16 ++++++++++++++++\n xdiff/xdiff.h     |  8 +++-----\n xdiff/xdiffi.c    | 20 ++++++++++----------\n xdiff/xinclude.h  |  2 +-\n xdiff/xmerge.c    |  4 ++--\n 5 files changed, 32 insertions(+), 18 deletions(-)\n create mode 100644 xdiff/git-xdiff.h\n\ndiff --git a/xdiff/git-xdiff.h b/xdiff/git-xdiff.h\nnew file mode 100644\nindex 0000000000..664a7c1351\n--- /dev/null\n+++ b/xdiff/git-xdiff.h\n@@ -0,0 +1,16 @@\n+#ifndef GIT_XDIFF_H\n+#define GIT_XDIFF_H\n+\n+#include \"git-compat-util.h\"\n+\n+#define xdl_malloc(x) xmalloc(x)\n+#define xdl_free(ptr) free(ptr)\n+#define xdl_realloc(ptr,x) xrealloc(ptr,x)\n+\n+#define xdl_regex_t regex_t\n+#define xdl_regmatch_t regmatch_t\n+#define xdl_regexec_buf(p, b, s, n, m, f) regexec_buf(p, b, s, n, m, f)\n+\n+#define XDL_BUG(msg) BUG(msg)\n+\n+#endif\ndiff --git a/xdiff/xdiff.h b/xdiff/xdiff.h\nindex 72e25a9ffa..fb47f63fbf 100644\n--- a/xdiff/xdiff.h\n+++ b/xdiff/xdiff.h\n@@ -27,6 +27,8 @@\n extern \"C\" {\n #endif /* #ifdef __cplusplus */\n\n+#include \"git-xdiff.h\"\n+\n /* xpparm_t.flags */\n #define XDF_NEED_MINIMAL (1 << 0)\n\n@@ -82,7 +84,7 @@ typedef struct s_xpparam {\n \tunsigned long flags;\n\n \t/* -I<regex> */\n-\tregex_t **ignore_regex;\n+\txdl_regex_t **ignore_regex;\n \tsize_t ignore_regex_nr;\n\n \t/* See Documentation/diff-options.txt. */\n@@ -119,10 +121,6 @@ typedef struct s_bdiffparam {\n } bdiffparam_t;\n\n\n-#define xdl_malloc(x) xmalloc(x)\n-#define xdl_free(ptr) free(ptr)\n-#define xdl_realloc(ptr,x) xrealloc(ptr,x)\n-\n void *xdl_mmfile_first(mmfile_t *mmf, long *size);\n long xdl_mmfile_size(mmfile_t *mmf);\n\ndiff --git a/xdiff/xdiffi.c b/xdiff/xdiffi.c\nindex 69689fab24..af31b7f4b3 100644\n--- a/xdiff/xdiffi.c\n+++ b/xdiff/xdiffi.c\n@@ -832,7 +832,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))\n \t\t\t\tif (group_previous(xdfo, &go))\n-\t\t\t\t\tBUG(\"group sync broken sliding up\");\n+\t\t\t\t\tXDL_BUG(\"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@@ -848,7 +848,7 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {\n \t\t\t\tif (group_slide_down(xdf, &g))\n \t\t\t\t\tbreak;\n \t\t\t\tif (group_next(xdfo, &go))\n-\t\t\t\t\tBUG(\"group sync broken sliding down\");\n+\t\t\t\t\tXDL_BUG(\"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@@ -873,9 +873,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))\n-\t\t\t\t\tBUG(\"match disappeared\");\n+\t\t\t\t\tXDL_BUG(\"match disappeared\");\n \t\t\t\tif (group_previous(xdfo, &go))\n-\t\t\t\t\tBUG(\"group sync broken sliding to match\");\n+\t\t\t\t\tXDL_BUG(\"group sync broken sliding to match\");\n \t\t\t}\n \t\t} else if (flags & XDF_INDENT_HEURISTIC) {\n \t\t\t/*\n@@ -916,9 +916,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))\n-\t\t\t\t\tBUG(\"best shift unreached\");\n+\t\t\t\t\tXDL_BUG(\"best shift unreached\");\n \t\t\t\tif (group_previous(xdfo, &go))\n-\t\t\t\t\tBUG(\"group sync broken sliding to blank line\");\n+\t\t\t\t\tXDL_BUG(\"group sync broken sliding to blank line\");\n \t\t\t}\n \t\t}\n\n@@ -927,11 +927,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\tBUG(\"group sync broken moving to next group\");\n+\t\t\tXDL_BUG(\"group sync broken moving to next group\");\n \t}\n\n \tif (!group_next(xdfo, &go))\n-\t\tBUG(\"group sync broken at end of file\");\n+\t\tXDL_BUG(\"group sync broken at end of file\");\n\n \treturn 0;\n }\n@@ -1011,11 +1011,11 @@ static void xdl_mark_ignorable_lines(xdchange_t *xscr, xdfenv_t *xe, long flags)\n }\n\n static int record_matches_regex(xrecord_t *rec, xpparam_t const *xpp) {\n-\tregmatch_t regmatch;\n+\txdl_regmatch_t regmatch;\n \tint i;\n\n \tfor (i = 0; i < xpp->ignore_regex_nr; i++)\n-\t\tif (!regexec_buf(xpp->ignore_regex[i], rec->ptr, rec->size, 1,\n+\t\tif (!xdl_regexec_buf(xpp->ignore_regex[i], rec->ptr, rec->size, 1,\n \t\t\t\t &regmatch, 0))\n \t\t\treturn 1;\n\ndiff --git a/xdiff/xinclude.h b/xdiff/xinclude.h\nindex a4285ac0eb..75db1d8f35 100644\n--- a/xdiff/xinclude.h\n+++ b/xdiff/xinclude.h\n@@ -23,7 +23,7 @@\n #if !defined(XINCLUDE_H)\n #define XINCLUDE_H\n\n-#include \"git-compat-util.h\"\n+#include \"git-xdiff.h\"\n #include \"xmacros.h\"\n #include \"xdiff.h\"\n #include \"xtypes.h\"\ndiff --git a/xdiff/xmerge.c b/xdiff/xmerge.c\nindex fff0b594f9..433e2d7415 100644\n--- a/xdiff/xmerge.c\n+++ b/xdiff/xmerge.c\n@@ -88,7 +88,7 @@ static int xdl_cleanup_merge(xdmerge_t *c)\n \t\tif (c->mode == 0)\n \t\t\tcount++;\n \t\tnext_c = c->next;\n-\t\tfree(c);\n+\t\txdl_free(c);\n \t}\n \treturn count;\n }\n@@ -456,7 +456,7 @@ static void xdl_merge_two_conflicts(xdmerge_t *m)\n \tm->chg1 = next_m->i1 + next_m->chg1 - m->i1;\n \tm->chg2 = next_m->i2 + next_m->chg2 - m->i2;\n \tm->next = next_m->next;\n-\tfree(next_m);\n+\txdl_free(next_m);\n }\n\n /*\n--\n2.35.1\n"},{"id":"449110","messageId":"e73c6746-9f8d-7e23-3764-18d01307278b@gmail.com","threadId":"57433","inReplyTo":"20220217225408.GB7@edef91d97c94","subject":"Re: [PATCH v2 1/1] xdiff: provide indirection to git functions","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2022-02-22T11:14:01Z","receivedAt":"2022-02-22T11:14:08Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 17/02/2022 22:54, Edward Thomson wrote:\n> Provide an indirection layer into the git-specific functionality and\n> utilities in `git-xdiff.h`, prefixing those types and functions with\n> `xdl_` (and `XDL_` for macros).  This allows other projects that use\n> git's xdiff implementation to keep up-to-date; they can now take all the\n> files _except_ `git-xdiff.h`, which they have customized for their own\n> environment.\n\nThe changes since V1 look good,\n\nBest Wishes\n\nPhillip\n\n> Signed-off-by: Edward Thomson <ethomson@edwardthomson.com>\n> ---\n>   xdiff/git-xdiff.h | 16 ++++++++++++++++\n>   xdiff/xdiff.h     |  8 +++-----\n>   xdiff/xdiffi.c    | 20 ++++++++++----------\n>   xdiff/xinclude.h  |  2 +-\n>   xdiff/xmerge.c    |  4 ++--\n>   5 files changed, 32 insertions(+), 18 deletions(-)\n>   create mode 100644 xdiff/git-xdiff.h\n> \n> diff --git a/xdiff/git-xdiff.h b/xdiff/git-xdiff.h\n> new file mode 100644\n> index 0000000000..664a7c1351\n> --- /dev/null\n> +++ b/xdiff/git-xdiff.h\n> @@ -0,0 +1,16 @@\n> +#ifndef GIT_XDIFF_H\n> +#define GIT_XDIFF_H\n> +\n> +#include \"git-compat-util.h\"\n> +\n> +#define xdl_malloc(x) xmalloc(x)\n> +#define xdl_free(ptr) free(ptr)\n> +#define xdl_realloc(ptr,x) xrealloc(ptr,x)\n> +\n> +#define xdl_regex_t regex_t\n> +#define xdl_regmatch_t regmatch_t\n> +#define xdl_regexec_buf(p, b, s, n, m, f) regexec_buf(p, b, s, n, m, f)\n> +\n> +#define XDL_BUG(msg) BUG(msg)\n> +\n> +#endif\n> diff --git a/xdiff/xdiff.h b/xdiff/xdiff.h\n> index 72e25a9ffa..fb47f63fbf 100644\n> --- a/xdiff/xdiff.h\n> +++ b/xdiff/xdiff.h\n> @@ -27,6 +27,8 @@\n>   extern \"C\" {\n>   #endif /* #ifdef __cplusplus */\n> \n> +#include \"git-xdiff.h\"\n> +\n>   /* xpparm_t.flags */\n>   #define XDF_NEED_MINIMAL (1 << 0)\n> \n> @@ -82,7 +84,7 @@ typedef struct s_xpparam {\n>   \tunsigned long flags;\n> \n>   \t/* -I<regex> */\n> -\tregex_t **ignore_regex;\n> +\txdl_regex_t **ignore_regex;\n>   \tsize_t ignore_regex_nr;\n> \n>   \t/* See Documentation/diff-options.txt. */\n> @@ -119,10 +121,6 @@ typedef struct s_bdiffparam {\n>   } bdiffparam_t;\n> \n> \n> -#define xdl_malloc(x) xmalloc(x)\n> -#define xdl_free(ptr) free(ptr)\n> -#define xdl_realloc(ptr,x) xrealloc(ptr,x)\n> -\n>   void *xdl_mmfile_first(mmfile_t *mmf, long *size);\n>   long xdl_mmfile_size(mmfile_t *mmf);\n> \n> diff --git a/xdiff/xdiffi.c b/xdiff/xdiffi.c\n> index 69689fab24..af31b7f4b3 100644\n> --- a/xdiff/xdiffi.c\n> +++ b/xdiff/xdiffi.c\n> @@ -832,7 +832,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))\n>   \t\t\t\tif (group_previous(xdfo, &go))\n> -\t\t\t\t\tBUG(\"group sync broken sliding up\");\n> +\t\t\t\t\tXDL_BUG(\"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> @@ -848,7 +848,7 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {\n>   \t\t\t\tif (group_slide_down(xdf, &g))\n>   \t\t\t\t\tbreak;\n>   \t\t\t\tif (group_next(xdfo, &go))\n> -\t\t\t\t\tBUG(\"group sync broken sliding down\");\n> +\t\t\t\t\tXDL_BUG(\"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> @@ -873,9 +873,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))\n> -\t\t\t\t\tBUG(\"match disappeared\");\n> +\t\t\t\t\tXDL_BUG(\"match disappeared\");\n>   \t\t\t\tif (group_previous(xdfo, &go))\n> -\t\t\t\t\tBUG(\"group sync broken sliding to match\");\n> +\t\t\t\t\tXDL_BUG(\"group sync broken sliding to match\");\n>   \t\t\t}\n>   \t\t} else if (flags & XDF_INDENT_HEURISTIC) {\n>   \t\t\t/*\n> @@ -916,9 +916,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))\n> -\t\t\t\t\tBUG(\"best shift unreached\");\n> +\t\t\t\t\tXDL_BUG(\"best shift unreached\");\n>   \t\t\t\tif (group_previous(xdfo, &go))\n> -\t\t\t\t\tBUG(\"group sync broken sliding to blank line\");\n> +\t\t\t\t\tXDL_BUG(\"group sync broken sliding to blank line\");\n>   \t\t\t}\n>   \t\t}\n> \n> @@ -927,11 +927,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\tBUG(\"group sync broken moving to next group\");\n> +\t\t\tXDL_BUG(\"group sync broken moving to next group\");\n>   \t}\n> \n>   \tif (!group_next(xdfo, &go))\n> -\t\tBUG(\"group sync broken at end of file\");\n> +\t\tXDL_BUG(\"group sync broken at end of file\");\n> \n>   \treturn 0;\n>   }\n> @@ -1011,11 +1011,11 @@ static void xdl_mark_ignorable_lines(xdchange_t *xscr, xdfenv_t *xe, long flags)\n>   }\n> \n>   static int record_matches_regex(xrecord_t *rec, xpparam_t const *xpp) {\n> -\tregmatch_t regmatch;\n> +\txdl_regmatch_t regmatch;\n>   \tint i;\n> \n>   \tfor (i = 0; i < xpp->ignore_regex_nr; i++)\n> -\t\tif (!regexec_buf(xpp->ignore_regex[i], rec->ptr, rec->size, 1,\n> +\t\tif (!xdl_regexec_buf(xpp->ignore_regex[i], rec->ptr, rec->size, 1,\n>   \t\t\t\t &regmatch, 0))\n>   \t\t\treturn 1;\n> \n> diff --git a/xdiff/xinclude.h b/xdiff/xinclude.h\n> index a4285ac0eb..75db1d8f35 100644\n> --- a/xdiff/xinclude.h\n> +++ b/xdiff/xinclude.h\n> @@ -23,7 +23,7 @@\n>   #if !defined(XINCLUDE_H)\n>   #define XINCLUDE_H\n> \n> -#include \"git-compat-util.h\"\n> +#include \"git-xdiff.h\"\n>   #include \"xmacros.h\"\n>   #include \"xdiff.h\"\n>   #include \"xtypes.h\"\n> diff --git a/xdiff/xmerge.c b/xdiff/xmerge.c\n> index fff0b594f9..433e2d7415 100644\n> --- a/xdiff/xmerge.c\n> +++ b/xdiff/xmerge.c\n> @@ -88,7 +88,7 @@ static int xdl_cleanup_merge(xdmerge_t *c)\n>   \t\tif (c->mode == 0)\n>   \t\t\tcount++;\n>   \t\tnext_c = c->next;\n> -\t\tfree(c);\n> +\t\txdl_free(c);\n>   \t}\n>   \treturn count;\n>   }\n> @@ -456,7 +456,7 @@ static void xdl_merge_two_conflicts(xdmerge_t *m)\n>   \tm->chg1 = next_m->i1 + next_m->chg1 - m->i1;\n>   \tm->chg2 = next_m->i2 + next_m->chg2 - m->i2;\n>   \tm->next = next_m->next;\n> -\tfree(next_m);\n> +\txdl_free(next_m);\n>   }\n> \n>   /*\n> --\n> 2.35.1\n\n"},{"id":"449589","messageId":"nycvar.QRO.7.76.6.2202251639590.11118@tvgsbejvaqbjf.bet","threadId":"57433","inReplyTo":"e73c6746-9f8d-7e23-3764-18d01307278b@gmail.com","subject":"Re: [PATCH v2 1/1] xdiff: provide indirection to git functions","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2022-02-25T15:41:57Z","receivedAt":"2022-02-25T15:42:12Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 22 Feb 2022, Phillip Wood wrote:\n\n> On 17/02/2022 22:54, Edward Thomson wrote:\n> > Provide an indirection layer into the git-specific functionality and\n> > utilities in `git-xdiff.h`, prefixing those types and functions with\n> > `xdl_` (and `XDL_` for macros).  This allows other projects that use\n> > git's xdiff implementation to keep up-to-date; they can now take all the\n> > files _except_ `git-xdiff.h`, which they have customized for their own\n> > environment.\n>\n> The changes since V1 look good,\n\nIndeed. This is the range-diff:\n\n-- snip --\n1:  52c8f141cbe1 ! 1:  e05e9b5e2f27 xdiff: provide indirection to git functions\n    @@ xdiff/git-xdiff.h (new)\n     +#ifndef GIT_XDIFF_H\n     +#define GIT_XDIFF_H\n     +\n    ++#include \"git-compat-util.h\"\n    ++\n     +#define xdl_malloc(x) xmalloc(x)\n     +#define xdl_free(ptr) free(ptr)\n     +#define xdl_realloc(ptr,x) xrealloc(ptr,x)\n    @@ xdiff/xdiffi.c: static void xdl_mark_ignorable_lines(xdchange_t *xscr, xdfenv_t\n\n      ## xdiff/xinclude.h ##\n     @@\n    + #if !defined(XINCLUDE_H)\n      #define XINCLUDE_H\n\n    - #include \"git-compat-util.h\"\n    +-#include \"git-compat-util.h\"\n     +#include \"git-xdiff.h\"\n      #include \"xmacros.h\"\n      #include \"xdiff.h\"\n      #include \"xtypes.h\"\n    -@@\n    - #include \"xdiffi.h\"\n    - #include \"xemit.h\"\n    +\n    + ## xdiff/xmerge.c ##\n    +@@ xdiff/xmerge.c: static int xdl_cleanup_merge(xdmerge_t *c)\n    + \t\tif (c->mode == 0)\n    + \t\t\tcount++;\n    + \t\tnext_c = c->next;\n    +-\t\tfree(c);\n    ++\t\txdl_free(c);\n    + \t}\n    + \treturn count;\n    + }\n    +@@ xdiff/xmerge.c: static void xdl_merge_two_conflicts(xdmerge_t *m)\n    + \tm->chg1 = next_m->i1 + next_m->chg1 - m->i1;\n    + \tm->chg2 = next_m->i2 + next_m->chg2 - m->i2;\n    + \tm->next = next_m->next;\n    +-\tfree(next_m);\n    ++\txdl_free(next_m);\n    + }\n\n    --\n    - #endif /* #if !defined(XINCLUDE_H) */\n    + /*\n-- snap --\n\nMy ACK from\nhttps://lore.kernel.org/git/nycvar.QRO.7.76.6.2202171644090.348@tvgsbejvaqbjf.bet/\nstill holds. Junio could you please add it before merging it down to\n`next`?\n\nThanks,\nDscho\n"},{"id":"449617","messageId":"xmqqo82udctt.fsf@gitster.g","threadId":"57433","inReplyTo":"nycvar.QRO.7.76.6.2202251639590.11118@tvgsbejvaqbjf.bet","subject":"Re: [PATCH v2 1/1] xdiff: provide indirection to git functions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-02-25T18:24:14Z","receivedAt":"2022-02-25T18:24:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> Hi,\n>\n> On Tue, 22 Feb 2022, Phillip Wood wrote:\n>\n>> On 17/02/2022 22:54, Edward Thomson wrote:\n>> > Provide an indirection layer into the git-specific functionality and\n>> > utilities in `git-xdiff.h`, prefixing those types and functions with\n>> > `xdl_` (and `XDL_` for macros).  This allows other projects that use\n>> > git's xdiff implementation to keep up-to-date; they can now take all the\n>> > files _except_ `git-xdiff.h`, which they have customized for their own\n>> > environment.\n>>\n>> The changes since V1 look good,\n>\n> Indeed. This is the range-diff:\n>\n> -- snip --\n> 1:  52c8f141cbe1 ! 1:  e05e9b5e2f27 xdiff: provide indirection to git functions\n>     @@ xdiff/git-xdiff.h (new)\n>      +#ifndef GIT_XDIFF_H\n>      +#define GIT_XDIFF_H\n>      +\n>     ++#include \"git-compat-util.h\"\n>     ++\n>      +#define xdl_malloc(x) xmalloc(x)\n>      +#define xdl_free(ptr) free(ptr)\n>      +#define xdl_realloc(ptr,x) xrealloc(ptr,x)\n>     @@ xdiff/xdiffi.c: static void xdl_mark_ignorable_lines(xdchange_t *xscr, xdfenv_t\n>\n>       ## xdiff/xinclude.h ##\n>      @@\n>     + #if !defined(XINCLUDE_H)\n>       #define XINCLUDE_H\n>\n>     - #include \"git-compat-util.h\"\n>     +-#include \"git-compat-util.h\"\n>      +#include \"git-xdiff.h\"\n>       #include \"xmacros.h\"\n>       #include \"xdiff.h\"\n>       #include \"xtypes.h\"\n>     -@@\n>     - #include \"xdiffi.h\"\n>     - #include \"xemit.h\"\n>     +\n>     + ## xdiff/xmerge.c ##\n>     +@@ xdiff/xmerge.c: static int xdl_cleanup_merge(xdmerge_t *c)\n>     + \t\tif (c->mode == 0)\n>     + \t\t\tcount++;\n>     + \t\tnext_c = c->next;\n>     +-\t\tfree(c);\n>     ++\t\txdl_free(c);\n>     + \t}\n>     + \treturn count;\n>     + }\n>     +@@ xdiff/xmerge.c: static void xdl_merge_two_conflicts(xdmerge_t *m)\n>     + \tm->chg1 = next_m->i1 + next_m->chg1 - m->i1;\n>     + \tm->chg2 = next_m->i2 + next_m->chg2 - m->i2;\n>     + \tm->next = next_m->next;\n>     +-\tfree(next_m);\n>     ++\txdl_free(next_m);\n>     + }\n>\n>     --\n>     - #endif /* #if !defined(XINCLUDE_H) */\n>     + /*\n> -- snap --\n>\n> My ACK from\n> https://lore.kernel.org/git/nycvar.QRO.7.76.6.2202171644090.348@tvgsbejvaqbjf.bet/\n> still holds. Junio could you please add it before merging it down to\n> `next`?\n\nNot so fast.  I still do not see a strong reason to support\nxdl_malloc() and other wrappers.\n\nIs the expectation for other projects when using the unified code,\nthey do not use xdiff/git-xdiff.h and instead add\nxdiff/frotz-xdiff.h that defines xdl_malloc() and friends with the\ninfrastructure they provide as part of the Frotz project (and the\nXyzzy project would do the same with xdiff/xyzzy-xdiff.h header for\nthem), making \"git\" the first among equal other consumers?\n\nIf that is the direction this indirection is aiming for, stating it\nclearly may be a start of a not-so-bad justification, but then the\nhardcoded inclusion of \"git-xdiff.h\" in xdiff/xinclude.h still\ncontradicts with it, which may want to be fixed.\n\n\n"},{"id":"449618","messageId":"20220225183854.GA9@811aa366e12e","threadId":"57433","inReplyTo":"xmqqo82udctt.fsf@gitster.g","subject":"Re: [PATCH v2 1/1] xdiff: provide indirection to git functions","fromName":"Edward Thomson","fromEmail":"ethomson@edwardthomson.com","sentAt":"2022-02-25T18:38:54Z","receivedAt":"2022-02-25T18:39:02Z","isPatch":true,"sender":{"key":"ethomson@edwardthomson.com","avatar":"https://avatars.githubusercontent.com/u/1130014?v=4"},"body":"On Fri, Feb 25, 2022 at 10:24:14AM -0800, Junio C Hamano wrote:\n> \n> Not so fast.  I still do not see a strong reason to support\n> xdl_malloc() and other wrappers.\n\ngit has an `xmalloc` but no matching `xfree`.  libgit2 does not\nnecessarily use the system allocator (and on Windows, you run into the\nquestion of _which_ system allocator you're using) and therefore has its\nown allocation _and_ deallocation functions.\n\nWhen libgit2 includes xdiff, I don't want to monkey around and try to\nredefine `free` to our deallocator.\n\nThere are several options that could suffice for this.  A different\ntactic is to have xdiff call `xfree` which is just defined as `free` in\ngit.  This would feel non-obvious to me as a git developer that in this\none part of the project, I need to use `xfree` instead of `free` on\nmemory that I have `xmalloc`ed.  Using a net new name for allocation\nfunctions may help serve as a reminder that it is a different API.\n\n> Is the expectation for other projects when using the unified code,\n> they do not use xdiff/git-xdiff.h and instead add\n> xdiff/frotz-xdiff.h that defines xdl_malloc() and friends with the\n> infrastructure they provide as part of the Frotz project (and the\n> Xyzzy project would do the same with xdiff/xyzzy-xdiff.h header for\n> them), making \"git\" the first among equal other consumers?\n\nNo, the thinking is that they would provide their own `git-xdiff.h` that\ndefines the mappings to their project-specific APIs.\n\nCheers-\n-ed\n"},{"id":"449623","messageId":"xmqqbkyudb8n.fsf@gitster.g","threadId":"57433","inReplyTo":"20220225183854.GA9@811aa366e12e","subject":"Re: [PATCH v2 1/1] xdiff: provide indirection to git functions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-02-25T18:58:32Z","receivedAt":"2022-02-25T18:58:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Edward Thomson <ethomson@edwardthomson.com> writes:\n\n> No, the thinking is that they would provide their own `git-xdiff.h` that\n> defines the mappings to their project-specific APIs.\n\nIs that spelled out somewhere?  That would help future readers of\nthe file to learn what they need to do when reusing the part,\nperhaps in a comment near the top of that file itself.\n\nIf git-xdiff.h is meant to be modified to match the need for non-git\ncodebase, it probably should be named to a more descriptive name,\nlike xdiff-compat.h or something, I would think.  git-xdiff.h that\nhas libgit2 specific names in it would look quite strange.\n\n"},{"id":"449624","messageId":"xmqq4k4mdb0i.fsf@gitster.g","threadId":"57433","inReplyTo":"20220217225408.GB7@edef91d97c94","subject":"Re: [PATCH v2 1/1] xdiff: provide indirection to git functions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-02-25T19:03:25Z","receivedAt":"2022-02-25T19:03:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Edward Thomson <ethomson@edwardthomson.com> writes:\n\n> Provide an indirection layer into the git-specific functionality and\n> utilities in `git-xdiff.h`, prefixing those types and functions with\n> `xdl_` (and `XDL_` for macros).  This allows other projects that use\n> git's xdiff implementation to keep up-to-date; they can now take all the\n> files _except_ `git-xdiff.h`, which they have customized for their own\n> environment.\n\nContinuing the \"what do they exactly do\" line of thought, the above\nis not quite in line with what I heard.  They take all the files\nincluding git-xdiff.h and they must modify git-xdiff.h to match\ntheir environment.\n\nIn any case, ...\n\n> diff --git a/xdiff/git-xdiff.h b/xdiff/git-xdiff.h\n> new file mode 100644\n> index 0000000000..664a7c1351\n> --- /dev/null\n> +++ b/xdiff/git-xdiff.h\n> @@ -0,0 +1,16 @@\n> +#ifndef GIT_XDIFF_H\n> +#define GIT_XDIFF_H\n\n... here is a good place to spell the expectation out, i.e. that\nthey are expected to change this file to match their system, and\nthat all the things they see below here (including the inclusion of\ngit-compat-util.h) is specific to git-core they are expected to rip\nout and replace.\n\n> +\n> +#include \"git-compat-util.h\"\n> +\n> +#define xdl_malloc(x) xmalloc(x)\n> +#define xdl_free(ptr) free(ptr)\n> +#define xdl_realloc(ptr,x) xrealloc(ptr,x)\n> +\n> +#define xdl_regex_t regex_t\n> +#define xdl_regmatch_t regmatch_t\n> +#define xdl_regexec_buf(p, b, s, n, m, f) regexec_buf(p, b, s, n, m, f)\n> +\n> +#define XDL_BUG(msg) BUG(msg)\n> +\n> +#endif\n\nThanks.\n"}]}