{"thread":{"id":"64199","subject":"[PATCH] diff --no-index: fix logic for paths ending in '/'","startedAt":"2025-09-24T20:57:35Z","lastAt":"2025-10-13T23:14:49Z","messageCount":8,"participants":["Jacob Keller","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"527261","messageId":"20250924-jk-fix-no-index-path-with-slash-v1-1-6b2028c0de92@intel.com","threadId":"64199","inReplyTo":null,"subject":"[PATCH] diff --no-index: fix logic for paths ending in '/'","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2025-09-24T20:57:15Z","receivedAt":"2025-09-24T20:57:35Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"If one of the two provided paths for git diff --no-index ends in a '/',\na failure similar to the following occurs:\n\n  $ git diff --no-index -- /tmp/ /tmp/ ':!'\n  fatal: `pos + len' is too far after the end of the buffer\n\nThis occurs because of an incorrect calculation of the skip lengths in\ndiff_no_index(). The code wants to calculate the length of the string,\nbut add one in case the string doesn't end with a slash.\n\nThe method it uses is incorrect, as it always checks the trailing NUL\ncharacter of the string. This will never be a '/', so we always add one.\nIn the event that we *do* have a trailing slash, this will create an\noff-by-one length error later when using the skip value.\n\nThe most straightforward fix would be to correct the skip1 and skip2\nlengths by using ends_with().\n\nHowever, Johannes made a good point that the existing logic is wasting a\nlot of computation. We generate the match string by copying the path in\nand then skipping almost all of it immediately with a potentially\nexpensive memmove() from the strbuf_remove() call. We also re-initialize\nthe match stringbuf each time we call read_directory_contents.\n\nThe read_directory_contents really wants a path that is rooted at the\nstart of the directory scan. We're currently building this by taking the\nfull path and stripping out the start portion. Instead, replace this\nlogic by building up the portion of the match as we go.\n\nStart by initializing two strbuf in diff_no_index containing the empty\nstring. Pass these into queue_diff, which in turn passes the appropriate\nleft or right side into read_directory_contents.\n\nAs before, we build up the matches by appending elements to the match\npath and then clearing them using strbuf_setlen.\n\nIn the recursive portion of the queue_diff algorithm, we build up new\nmatch paths the same way that we build up new buffer paths, by appending\nthe elements and then clearing them with strbuf_setlen after each\niteration. This is cheaper as it avoids repeated allocations, and is a\nbit simpler to track what is going on.\n\nAdd a couple of test cases that pass in paths already ending in '/', to\nensure the tests cover this regression.\n\nReported-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>\nCloses: https://lore.kernel.org/git/c75ec5f9-407a-6555-d4fb-bb629d54ec61@gmx.de/\nSigned-off-by: Jacob Keller <jacob.e.keller@intel.com>\n---\nThis fix might feel overly complex. We can drop this and just go with a\nsimple ends_with() fix, but that leaves the needless strbuf_remove() in the\nread_directory_contents.\n\nI tried some alternatives in that function, including trying to use\n&path[skip], but this doesn't quite work since path might not end with a\nslash, while the skip length does need to account for the possible slash,\nsince we don't want to end up accidentally using an absolute path.\n---\n diff-no-index.c          | 61 ++++++++++++++++++++++++++----------------------\n t/t4053-diff-no-index.sh | 16 +++++++++++++\n 2 files changed, 49 insertions(+), 28 deletions(-)\n\ndiff --git a/diff-no-index.c b/diff-no-index.c\nindex 88ae4cee56ba..c70f82b80559 100644\n--- a/diff-no-index.c\n+++ b/diff-no-index.c\n@@ -21,30 +21,21 @@\n \n static int read_directory_contents(const char *path, struct string_list *list,\n \t\t\t\t   const struct pathspec *pathspec,\n-\t\t\t\t   int skip)\n+\t\t\t\t   struct strbuf *match)\n {\n-\tstruct strbuf match = STRBUF_INIT;\n-\tint len;\n+\tint len = match->len;\n \tDIR *dir;\n \tstruct dirent *e;\n \n \tif (!(dir = opendir(path)))\n \t\treturn error(\"Could not open directory %s\", path);\n \n-\tif (pathspec) {\n-\t\tstrbuf_addstr(&match, path);\n-\t\tstrbuf_complete(&match, '/');\n-\t\tstrbuf_remove(&match, 0, skip);\n-\n-\t\tlen = match.len;\n-\t}\n-\n \twhile ((e = readdir_skip_dot_and_dotdot(dir))) {\n \t\tif (pathspec) {\n \t\t\tint is_dir = 0;\n \n-\t\t\tstrbuf_setlen(&match, len);\n-\t\t\tstrbuf_addstr(&match, e->d_name);\n+\t\t\tstrbuf_setlen(match, len);\n+\t\t\tstrbuf_addstr(match, e->d_name);\n \t\t\tif (NOT_CONSTANT(DTYPE(e)) != DT_UNKNOWN) {\n \t\t\t\tis_dir = (DTYPE(e) == DT_DIR);\n \t\t\t} else {\n@@ -57,7 +48,7 @@ static int read_directory_contents(const char *path, struct string_list *list,\n \t\t\t}\n \n \t\t\tif (!match_leading_pathspec(NULL, pathspec,\n-\t\t\t\t\t\t    match.buf, match.len,\n+\t\t\t\t\t\t    match->buf, match->len,\n \t\t\t\t\t\t    0, NULL, is_dir))\n \t\t\t\tcontinue;\n \t\t}\n@@ -65,7 +56,7 @@ static int read_directory_contents(const char *path, struct string_list *list,\n \t\tstring_list_insert(list, e->d_name);\n \t}\n \n-\tstrbuf_release(&match);\n+\tstrbuf_setlen(match, len);\n \tclosedir(dir);\n \treturn 0;\n }\n@@ -169,7 +160,8 @@ static struct diff_filespec *noindex_filespec(const struct git_hash_algo *algop,\n \n static int queue_diff(struct diff_options *o, const struct git_hash_algo *algop,\n \t\t      const char *name1, const char *name2, int recursing,\n-\t\t      const struct pathspec *ps, int skip1, int skip2)\n+\t\t      const struct pathspec *ps,\n+\t\t      struct strbuf *ps_match1, struct strbuf *ps_match2)\n {\n \tint mode1 = 0, mode2 = 0;\n \tenum special special1 = SPECIAL_NONE, special2 = SPECIAL_NONE;\n@@ -208,10 +200,12 @@ static int queue_diff(struct diff_options *o, const struct git_hash_algo *algop,\n \t\tstruct string_list p2 = STRING_LIST_INIT_DUP;\n \t\tint i1, i2, ret = 0;\n \t\tsize_t len1 = 0, len2 = 0;\n+\t\tsize_t match1_len = ps_match1->len;\n+\t\tsize_t match2_len = ps_match2->len;\n \n-\t\tif (name1 && read_directory_contents(name1, &p1, ps, skip1))\n+\t\tif (name1 && read_directory_contents(name1, &p1, ps, ps_match1))\n \t\t\treturn -1;\n-\t\tif (name2 && read_directory_contents(name2, &p2, ps, skip2)) {\n+\t\tif (name2 && read_directory_contents(name2, &p2, ps, ps_match2)) {\n \t\t\tstring_list_clear(&p1, 0);\n \t\t\treturn -1;\n \t\t}\n@@ -235,6 +229,11 @@ static int queue_diff(struct diff_options *o, const struct git_hash_algo *algop,\n \t\t\tstrbuf_setlen(&buffer1, len1);\n \t\t\tstrbuf_setlen(&buffer2, len2);\n \n+\t\t\tif (ps) {\n+\t\t\t\tstrbuf_setlen(ps_match1, match1_len);\n+\t\t\t\tstrbuf_setlen(ps_match2, match2_len);\n+\t\t\t}\n+\n \t\t\tif (i1 == p1.nr)\n \t\t\t\tcomp = 1;\n \t\t\telse if (i2 == p2.nr)\n@@ -245,18 +244,28 @@ static int queue_diff(struct diff_options *o, const struct git_hash_algo *algop,\n \t\t\tif (comp > 0)\n \t\t\t\tn1 = NULL;\n \t\t\telse {\n-\t\t\t\tstrbuf_addstr(&buffer1, p1.items[i1++].string);\n+\t\t\t\tstrbuf_addstr(&buffer1, p1.items[i1].string);\n+\t\t\t\tif (ps) {\n+\t\t\t\t\tstrbuf_addstr(ps_match1, p1.items[i1].string);\n+\t\t\t\t\tstrbuf_complete(ps_match1, '/');\n+\t\t\t\t}\n \t\t\t\tn1 = buffer1.buf;\n+\t\t\t\ti1++;\n \t\t\t}\n \n \t\t\tif (comp < 0)\n \t\t\t\tn2 = NULL;\n \t\t\telse {\n-\t\t\t\tstrbuf_addstr(&buffer2, p2.items[i2++].string);\n+\t\t\t\tstrbuf_addstr(&buffer2, p2.items[i2].string);\n+\t\t\t\tif (ps) {\n+\t\t\t\t\tstrbuf_addstr(ps_match2, p2.items[i2].string);\n+\t\t\t\t\tstrbuf_complete(ps_match2, '/');\n+\t\t\t\t}\n \t\t\t\tn2 = buffer2.buf;\n+\t\t\t\ti2++;\n \t\t\t}\n \n-\t\t\tret = queue_diff(o, algop, n1, n2, 1, ps, skip1, skip2);\n+\t\t\tret = queue_diff(o, algop, n1, n2, 1, ps, ps_match1, ps_match2);\n \t\t}\n \t\tstring_list_clear(&p1, 0);\n \t\tstring_list_clear(&p2, 0);\n@@ -346,7 +355,8 @@ int diff_no_index(struct rev_info *revs, const struct git_hash_algo *algop,\n \t\t  int implicit_no_index, int argc, const char **argv)\n {\n \tstruct pathspec pathspec, *ps = NULL;\n-\tint i, no_index, skip1 = 0, skip2 = 0;\n+\tstruct strbuf ps_match1 = STRBUF_INIT, ps_match2 = STRBUF_INIT;\n+\tint i, no_index;\n \tint ret = 1;\n \tconst char *paths[2];\n \tchar *to_free[ARRAY_SIZE(paths)] = { 0 };\n@@ -387,11 +397,6 @@ int diff_no_index(struct rev_info *revs, const struct git_hash_algo *algop,\n \t\t\t       NULL, &argv[2]);\n \t\tif (pathspec.nr)\n \t\t\tps = &pathspec;\n-\n-\t\tskip1 = strlen(paths[0]);\n-\t\tskip1 += paths[0][skip1] == '/' ? 0 : 1;\n-\t\tskip2 = strlen(paths[1]);\n-\t\tskip2 += paths[1][skip2] == '/' ? 0 : 1;\n \t} else if (argc > 2) {\n \t\twarning(_(\"Limiting comparison with pathspecs is only \"\n \t\t\t  \"supported if both paths are directories.\"));\n@@ -415,7 +420,7 @@ int diff_no_index(struct rev_info *revs, const struct git_hash_algo *algop,\n \trevs->diffopt.flags.exit_with_status = 1;\n \n \tif (queue_diff(&revs->diffopt, algop, paths[0], paths[1], 0, ps,\n-\t\t       skip1, skip2))\n+\t\t       &ps_match1, &ps_match2))\n \t\tgoto out;\n \tdiff_set_mnemonic_prefix(&revs->diffopt, \"1/\", \"2/\");\n \tdiffcore_std(&revs->diffopt);\ndiff --git a/t/t4053-diff-no-index.sh b/t/t4053-diff-no-index.sh\nindex 01db9243abfe..e0ea437685b0 100755\n--- a/t/t4053-diff-no-index.sh\n+++ b/t/t4053-diff-no-index.sh\n@@ -322,6 +322,22 @@ test_expect_success 'diff --no-index with pathspec' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'diff --no-index first path ending in slash with pathspec' '\n+\ttest_expect_code 1 git diff --name-status --no-index a/ b 1 >actual &&\n+\tcat >expect <<-EOF &&\n+\tD\ta/1\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'diff --no-index second path ending in slash with pathspec' '\n+\ttest_expect_code 1 git diff --name-status --no-index a b/ 1 >actual &&\n+\tcat >expect <<-EOF &&\n+\tD\ta/1\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'diff --no-index with pathspec no matches' '\n \ttest_expect_code 0 git diff --name-status --no-index a b missing\n '\n\n---\nbase-commit: 8d2709d075d65ba386a4dac157129ef868c283e5\nchange-id: 20250924-jk-fix-no-index-path-with-slash-788785be16c1\n\nBest regards,\n--  \nJacob Keller <jacob.e.keller@intel.com>\n\n"},{"id":"527272","messageId":"xmqqecrvjyoi.fsf@gitster.g","threadId":"64199","inReplyTo":"20250924-jk-fix-no-index-path-with-slash-v1-1-6b2028c0de92@intel.com","subject":"Re: [PATCH] diff --no-index: fix logic for paths ending in '/'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-09-24T22:03:25Z","receivedAt":"2025-09-24T22:03:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jacob Keller <jacob.e.keller@intel.com> writes:\n\n> If one of the two provided paths for git diff --no-index ends in a '/',\n> a failure similar to the following occurs:\n>\n>   $ git diff --no-index -- /tmp/ /tmp/ ':!'\n>   fatal: `pos + len' is too far after the end of the buffer\n>\n> This occurs because of an incorrect calculation of the skip lengths in\n> diff_no_index(). The code wants to calculate the length of the string,\n> but add one in case the string doesn't end with a slash.\n>\n> The method it uses is incorrect, as it always checks the trailing NUL\n> character of the string. This will never be a '/', so we always add one.\n> In the event that we *do* have a trailing slash, this will create an\n> off-by-one length error later when using the skip value.\n>\n> The most straightforward fix would be to correct the skip1 and skip2\n> lengths by using ends_with().\n\nMeaning, \"ah, this one ends with '/' so let's trim it and do\neverything else the same as before\"?\n\nIt certainly is how we usually handle regression fixes.\n\nThere were two topics that touched \"git diff --no-index\" in Git 2.51\ntimeframe, and the pathspec support was a new feature added by them,\nso this is not exactly a regression.\n\n> This fix might feel overly complex. We can drop this and just go with a\n> simple ends_with() fix, but that leaves the needless strbuf_remove() in the\n> read_directory_contents.\n\nWe can do it as a two-step patch series, which would allow us to\nrevert the more complex part relatively cleanly if it turns out to\nbreak the cases that the current code handles fine.  But as this is\na new feature in 2.51, it is OK to declare that the code never\nworked correctly and we are making it right this time with a single\nmore ambitious patch.\n\nWill queue.  Thanks, both of you.\n"},{"id":"527273","messageId":"xmqqa52jjxyq.fsf@gitster.g","threadId":"64199","inReplyTo":"20250924-jk-fix-no-index-path-with-slash-v1-1-6b2028c0de92@intel.com","subject":"Re: [PATCH] diff --no-index: fix logic for paths ending in '/'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-09-24T22:18:53Z","receivedAt":"2025-09-24T22:18:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jacob Keller <jacob.e.keller@intel.com> writes:\n\n> diff --git a/diff-no-index.c b/diff-no-index.c\n> index 88ae4cee56ba..c70f82b80559 100644\n> --- a/diff-no-index.c\n> +++ b/diff-no-index.c\n> ...\n> @@ -346,7 +355,8 @@ int diff_no_index(struct rev_info *revs, const struct git_hash_algo *algop,\n>  \t\t  int implicit_no_index, int argc, const char **argv)\n>  {\n>  \tstruct pathspec pathspec, *ps = NULL;\n> -\tint i, no_index, skip1 = 0, skip2 = 0;\n> +\tstruct strbuf ps_match1 = STRBUF_INIT, ps_match2 = STRBUF_INIT;\n> +\tint i, no_index;\n>  \tint ret = 1;\n>  \tconst char *paths[2];\n>  \tchar *to_free[ARRAY_SIZE(paths)] = { 0 };\n> @@ -387,11 +397,6 @@ int diff_no_index(struct rev_info *revs, const struct git_hash_algo *algop,\n>  \t\t\t       NULL, &argv[2]);\n>  \t\tif (pathspec.nr)\n>  \t\t\tps = &pathspec;\n> -\n> -\t\tskip1 = strlen(paths[0]);\n> -\t\tskip1 += paths[0][skip1] == '/' ? 0 : 1;\n> -\t\tskip2 = strlen(paths[1]);\n> -\t\tskip2 += paths[1][skip2] == '/' ? 0 : 1;\n>  \t} else if (argc > 2) {\n>  \t\twarning(_(\"Limiting comparison with pathspecs is only \"\n>  \t\t\t  \"supported if both paths are directories.\"));\n> @@ -415,7 +420,7 @@ int diff_no_index(struct rev_info *revs, const struct git_hash_algo *algop,\n>  \trevs->diffopt.flags.exit_with_status = 1;\n>  \n>  \tif (queue_diff(&revs->diffopt, algop, paths[0], paths[1], 0, ps,\n> -\t\t       skip1, skip2))\n> +\t\t       &ps_match1, &ps_match2))\n\nInside queue_diff() that makes recursive calls to itself, lenthens\nthese strbuf to hold longer paths while using setlen when it wants\nto trim the tail end of the paths.\n\nSo it is likely that ps_match.buf would never become NUL even when\nps_match.len goes down to 0 after the recursion and queue_diff()\nuses setlen to trim the string back to what was originally in there.\n\nHence, I think the clean-up code of this function this goto ...\n\n>  \t\tgoto out;\n\n... jumps to would need\n\n\tstrbuf_release(&ps_match1);\n\tstrbuf_release(&ps_match2);\n\nadded after that \"out:\" label?\n\nIf we run this test with leak sanitizer, wouldn't it find leak in\nthese (I haven't tried it myself---I just am speculating)?\n\n> diff --git a/t/t4053-diff-no-index.sh b/t/t4053-diff-no-index.sh\n> index 01db9243abfe..e0ea437685b0 100755\n> --- a/t/t4053-diff-no-index.sh\n> +++ b/t/t4053-diff-no-index.sh\n> @@ -322,6 +322,22 @@ test_expect_success 'diff --no-index with pathspec' '\n>  \ttest_cmp expect actual\n>  '\n>  \n> +test_expect_success 'diff --no-index first path ending in slash with pathspec' '\n> +\ttest_expect_code 1 git diff --name-status --no-index a/ b 1 >actual &&\n> +\tcat >expect <<-EOF &&\n> +\tD\ta/1\n> +\tEOF\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_expect_success 'diff --no-index second path ending in slash with pathspec' '\n> +\ttest_expect_code 1 git diff --name-status --no-index a b/ 1 >actual &&\n> +\tcat >expect <<-EOF &&\n> +\tD\ta/1\n> +\tEOF\n> +\ttest_cmp expect actual\n> +'\n> +\n>  test_expect_success 'diff --no-index with pathspec no matches' '\n>  \ttest_expect_code 0 git diff --name-status --no-index a b missing\n>  '\n"},{"id":"527274","messageId":"xmqq5xd7jxpq.fsf@gitster.g","threadId":"64199","inReplyTo":"xmqqa52jjxyq.fsf@gitster.g","subject":"Re: [PATCH] diff --no-index: fix logic for paths ending in '/'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-09-24T22:24:17Z","receivedAt":"2025-09-24T22:24:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Hence, I think the clean-up code of this function this goto ...\n>\n>>  \t\tgoto out;\n>\n> ... jumps to would need\n>\n> \tstrbuf_release(&ps_match1);\n> \tstrbuf_release(&ps_match2);\n>\n> added after that \"out:\" label?\n>\n> If we run this test with leak sanitizer, wouldn't it find leak in\n> these (I haven't tried it myself---I just am speculating)?\n\nNow I did, and my speculations were both correct.  The SANITIZE=leak\nbuild fails, and with these two releases the test passes.\n\nYou can squash this in, or I can do so myself if you like, if this\nis the only change that is required.\n\n diff-no-index.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git c/diff-no-index.c w/diff-no-index.c\nindex c70f82b805..f320424f05 100644\n--- c/diff-no-index.c\n+++ w/diff-no-index.c\n@@ -436,6 +436,8 @@ int diff_no_index(struct rev_info *revs, const struct git_hash_algo *algop,\n \tfor (i = 0; i < ARRAY_SIZE(to_free); i++)\n \t\tfree(to_free[i]);\n \tstrbuf_release(&replacement);\n+\tstrbuf_release(&ps_match1);\n+\tstrbuf_release(&ps_match2);\n \tif (ps)\n \t\tclear_pathspec(ps);\n \treturn ret;\n"},{"id":"527342","messageId":"de75e7f3-6c59-4f62-8d11-dea33804cc59@intel.com","threadId":"64199","inReplyTo":"xmqq5xd7jxpq.fsf@gitster.g","subject":"Re: [PATCH] diff --no-index: fix logic for paths ending in '/'","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2025-09-25T17:17:54Z","receivedAt":"2025-09-25T17:18:05Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"\n\nOn 9/24/2025 3:24 PM, Junio C Hamano wrote:\n> Junio C Hamano <gitster@pobox.com> writes:\n> \n>> Hence, I think the clean-up code of this function this goto ...\n>>\n>>>  \t\tgoto out;\n>>\n>> ... jumps to would need\n>>\n>> \tstrbuf_release(&ps_match1);\n>> \tstrbuf_release(&ps_match2);\n>>\n>> added after that \"out:\" label?\n>>\n>> If we run this test with leak sanitizer, wouldn't it find leak in\n>> these (I haven't tried it myself---I just am speculating)?\n> \n> Now I did, and my speculations were both correct.  The SANITIZE=leak\n> build fails, and with these two releases the test passes.\n> \n> You can squash this in, or I can do so myself if you like, if this\n> is the only change that is required.\n> \n>  diff-no-index.c | 2 ++\n>  1 file changed, 2 insertions(+)\n> \n> diff --git c/diff-no-index.c w/diff-no-index.c\n> index c70f82b805..f320424f05 100644\n> --- c/diff-no-index.c\n> +++ w/diff-no-index.c\n> @@ -436,6 +436,8 @@ int diff_no_index(struct rev_info *revs, const struct git_hash_algo *algop,\n>  \tfor (i = 0; i < ARRAY_SIZE(to_free); i++)\n>  \t\tfree(to_free[i]);\n>  \tstrbuf_release(&replacement);\n> +\tstrbuf_release(&ps_match1);\n> +\tstrbuf_release(&ps_match2);\n>  \tif (ps)\n>  \t\tclear_pathspec(ps);\n>  \treturn ret;\n\nPlease squash this in. I'll fix it if we need a v2 otherwise.\n"},{"id":"527347","messageId":"xmqqy0q2fkh8.fsf@gitster.g","threadId":"64199","inReplyTo":"de75e7f3-6c59-4f62-8d11-dea33804cc59@intel.com","subject":"Re: [PATCH] diff --no-index: fix logic for paths ending in '/'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-09-25T18:36:03Z","receivedAt":"2025-09-25T18:36:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jacob Keller <jacob.e.keller@intel.com> writes:\n\n>> Now I did, and my speculations were both correct.  The SANITIZE=leak\n>> build fails, and with these two releases the test passes.\n>> \n>> You can squash this in, or I can do so myself if you like, if this\n>> is the only change that is required.\n\nWill do.  Thanks.\n"},{"id":"528519","messageId":"xmqqjz12ag5j.fsf@gitster.g","threadId":"64199","inReplyTo":"de75e7f3-6c59-4f62-8d11-dea33804cc59@intel.com","subject":"Re: [PATCH] diff --no-index: fix logic for paths ending in '/'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-10-10T16:13:44Z","receivedAt":"2025-10-10T16:13:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jacob Keller <jacob.e.keller@intel.com> writes:\n\n>> You can squash this in, or I can do so myself if you like, if this\n>> is the only change that is required.\n>> \n>>  diff-no-index.c | 2 ++\n>>  1 file changed, 2 insertions(+)\n>> \n>> diff --git c/diff-no-index.c w/diff-no-index.c\n>> index c70f82b805..f320424f05 100644\n>> --- c/diff-no-index.c\n>> +++ w/diff-no-index.c\n>> @@ -436,6 +436,8 @@ int diff_no_index(struct rev_info *revs, const struct git_hash_algo *algop,\n>>  \tfor (i = 0; i < ARRAY_SIZE(to_free); i++)\n>>  \t\tfree(to_free[i]);\n>>  \tstrbuf_release(&replacement);\n>> +\tstrbuf_release(&ps_match1);\n>> +\tstrbuf_release(&ps_match2);\n>>  \tif (ps)\n>>  \t\tclear_pathspec(ps);\n>>  \treturn ret;\n>\n> Please squash this in. I'll fix it if we need a v2 otherwise.\n\nSince this exchange things have gone quiet, so shall we declare\nvictory and merge it down to 'next' and then eventually to 'master'?\n\nThanks.\n"},{"id":"528675","messageId":"0cc105dd-717a-49d1-96ca-e884e7d9f157@intel.com","threadId":"64199","inReplyTo":"xmqqjz12ag5j.fsf@gitster.g","subject":"Re: [PATCH] diff --no-index: fix logic for paths ending in '/'","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2025-10-13T23:14:45Z","receivedAt":"2025-10-13T23:14:49Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"\n\nOn 10/10/2025 9:13 AM, Junio C Hamano wrote:\n> Jacob Keller <jacob.e.keller@intel.com> writes:\n> \n>>> You can squash this in, or I can do so myself if you like, if this\n>>> is the only change that is required.\n>>>\n>>>  diff-no-index.c | 2 ++\n>>>  1 file changed, 2 insertions(+)\n>>>\n>>> diff --git c/diff-no-index.c w/diff-no-index.c\n>>> index c70f82b805..f320424f05 100644\n>>> --- c/diff-no-index.c\n>>> +++ w/diff-no-index.c\n>>> @@ -436,6 +436,8 @@ int diff_no_index(struct rev_info *revs, const struct git_hash_algo *algop,\n>>>  \tfor (i = 0; i < ARRAY_SIZE(to_free); i++)\n>>>  \t\tfree(to_free[i]);\n>>>  \tstrbuf_release(&replacement);\n>>> +\tstrbuf_release(&ps_match1);\n>>> +\tstrbuf_release(&ps_match2);\n>>>  \tif (ps)\n>>>  \t\tclear_pathspec(ps);\n>>>  \treturn ret;\n>>\n>> Please squash this in. I'll fix it if we need a v2 otherwise.\n> \n> Since this exchange things have gone quiet, so shall we declare\n> victory and merge it down to 'next' and then eventually to 'master'?\n> \n> Thanks.\n\nThat would be my preference. I haven't seen any other reports or\ncomments about this since the v1 besides the missing strbuf release.\n\nThanks,\nJake\n"}]}