{"thread":{"id":"66218","subject":"[PATCH 4/4] worktree add: let worktree_basename() return string copy","startedAt":"2026-08-25T18:04:04Z","lastAt":"2026-08-31T18:56:44Z","messageCount":10,"participants":["René Scharfe","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"551210","messageId":"20260825180350.2099-5-l.s.r@web.de","threadId":"66218","inReplyTo":"20260825180350.2099-1-l.s.r@web.de","subject":"[PATCH 4/4] worktree add: let worktree_basename() return string copy","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2026-08-25T18:03:50Z","receivedAt":"2026-08-25T18:04:04Z","isPatch":true,"body":"worktree_basename() requires callers to do pointer arithmetic to get the\nactual basename. Simplify them by doing the calculations in the\nfunction and returning a copy of the basename directly.\n\nRemind programmers to free the result by renaming the function to\nworktree_basename_dup(). Two already do; convert the remaining one from\nresetting a shared strbuf to freeing the allocated string, which\nrequires the same number of lines, but no arithmetic. The added\nallocation is negligible because it's small and there's only one per run\nof \"git worktree add\".\n\nSigned-off-by: René Scharfe \n---\n builtin/worktree.c | 25 ++++++++++---------------\n 1 file changed, 10 insertions(+), 15 deletions(-)\n\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex 01c245778e..d95824b2fd 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -294,7 +294,7 @@ static void remove_junk_on_signal(int signo)\n \traise(signo);\n }\n \n-static const char *worktree_basename(const char *path, int *olen)\n+static char *worktree_basename_dup(const char *path)\n {\n \tconst char *name;\n \tint len;\n@@ -307,8 +307,7 @@ static const char *worktree_basename(const char *path, int *olen)\n \twhile (name > path && !is_dir_sep(name[-1]))\n \t\tname--;\n \n-\t*olen = len;\n-\treturn name;\n+\treturn xmemdupz(name, path + len - name);\n }\n \n /* check that path is viable location for worktree */\n@@ -462,6 +461,7 @@ static int add_worktree(const char *path, const char *refname,\n \tstruct strbuf sb_git = STRBUF_INIT, sb_repo = STRBUF_INIT;\n \tstruct strbuf sb = STRBUF_INIT;\n \tconst char *name;\n+\tchar *name_to_free = NULL;\n \tstruct strvec child_env = STRVEC_INIT;\n \tunsigned int counter = 0;\n \tint len, ret;\n@@ -489,14 +489,12 @@ static int add_worktree(const char *path, const char *refname,\n \tif (!commit && !opts->orphan)\n \t\tdie(_(\"invalid reference: %s\"), refname);\n \n-\tname = worktree_basename(path, &len);\n-\tstrbuf_add(&sb, name, path + len - name);\n-\tif (!sb.len)\n+\tname = name_to_free = worktree_basename_dup(path);\n+\tif (!*name)\n \t\tdie(_(\"invalid path '%s'\"), path);\n-\tsanitize_refname_component(sb.buf, &sb_name);\n+\tsanitize_refname_component(name, &sb_name);\n \tif (!sb_name.len)\n-\t\tBUG(\"How come '%s' becomes empty after sanitization?\", sb.buf);\n-\tstrbuf_reset(&sb);\n+\t\tBUG(\"How come '%s' becomes empty after sanitization?\", name);\n \tname = sb_name.buf;\n \trepo_git_path_replace(the_repository, &sb_repo, \"worktrees/%s\", name);\n \tlen = sb_repo.len;\n@@ -630,6 +628,7 @@ static int add_worktree(const char *path, const char *refname,\n \tstrbuf_release(&sb_git);\n \tstrbuf_release(&sb_name);\n \tfree_worktree(wt);\n+\tfree(name_to_free);\n \treturn ret;\n }\n \n@@ -766,10 +765,8 @@ static int dwim_orphan(const struct add_opts *opts, int opt_track, int remote)\n \n static char *dwim_branch(const char *path, char **new_branch)\n {\n-\tint n;\n \tint branch_exists;\n-\tconst char *s = worktree_basename(path, &n);\n-\tchar *branchname = xmemdupz(s, path + n - s);\n+\tchar *branchname = worktree_basename_dup(path);\n \tstruct strbuf ref = STRBUF_INIT;\n \n \tbranch_exists = !check_branch_ref(the_repository, &ref, branchname) &&\n@@ -876,9 +873,7 @@ static int add(int ac, const char **av, const char *prefix,\n \t}\n \n \tif (opts.orphan && !new_branch) {\n-\t\tint n;\n-\t\tconst char *s = worktree_basename(path, &n);\n-\t\tnew_branch = new_branch_to_free = xmemdupz(s, path + n - s);\n+\t\tnew_branch = new_branch_to_free = worktree_basename_dup(path);\n \t} else if (opts.orphan) {\n \t\t; /* no-op */\n \t} else if (opts.detach) {\n-- \n2.55.0\n\n"},{"id":"551211","messageId":"20260825180350.2099-1-l.s.r@web.de","threadId":"66218","inReplyTo":null,"subject":"[PATCH 0/4] worktree add: worktree_basename() fixes","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2026-08-25T18:03:46Z","receivedAt":"2026-08-25T18:04:04Z","isPatch":true,"body":"Here's my take on solving the issues addressed by [1]. I'd prefer a\nreroll of that series, but in case it won't come we can use this one.\n\nThe first fix is the most important one, avoiding potential data loss.\nThe third one starts to accept paths with trailing path separators, the\nfourth one simplifies the code.\n\n worktree add: don't read out of bounds in worktree_basename()\n worktree add: reject separator-only path\n worktree add: trim slashes when deriving branch name from path\n worktree add: let worktree_basename() return string copy\n\n builtin/worktree.c | 33 ++++++++++++++-------------------\n t/t2400-worktree-add.sh | 17 +++++++++++++++++\n 2 files changed, 31 insertions(+), 19 deletions(-)\n\n\n[1] https://lore.kernel.org/git/pull.2187.git.1784978348.gitgitgadget@gmail.com/\n\n-- \n2.55.0\n\n"},{"id":"551212","messageId":"20260825180350.2099-4-l.s.r@web.de","threadId":"66218","inReplyTo":"20260825180350.2099-1-l.s.r@web.de","subject":"[PATCH 3/4] worktree add: trim slashes when deriving branch name from path","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2026-08-25T18:03:49Z","receivedAt":"2026-08-25T18:04:04Z","isPatch":true,"body":"worktree_basename() sets `n` to the length of `path` without trailing\npath separators, not to the length of the basename. This matters when\nderiving a branch name from a path with more than one component. E.g.:\n\n path: /new/worktree/\n s: ^\n n: |-----------|\n\nSo here xstrndup(s, n) copies up to 13 characters from \"worktree/\",\neffectively to the end of the string, including the trailing dash.\n\nPath separators are not allowed at the end of branch names, so strip\nthem off by calculating the basename length and extracting just that\npart.\n\nSigned-off-by: René Scharfe \n---\n builtin/worktree.c | 4 ++--\n t/t2400-worktree-add.sh | 13 +++++++++++++\n 2 files changed, 15 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex a53e815cc9..01c245778e 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -769,7 +769,7 @@ static char *dwim_branch(const char *path, char **new_branch)\n \tint n;\n \tint branch_exists;\n \tconst char *s = worktree_basename(path, &n);\n-\tchar *branchname = xstrndup(s, n);\n+\tchar *branchname = xmemdupz(s, path + n - s);\n \tstruct strbuf ref = STRBUF_INIT;\n \n \tbranch_exists = !check_branch_ref(the_repository, &ref, branchname) &&\n@@ -878,7 +878,7 @@ static int add(int ac, const char **av, const char *prefix,\n \tif (opts.orphan && !new_branch) {\n \t\tint n;\n \t\tconst char *s = worktree_basename(path, &n);\n-\t\tnew_branch = new_branch_to_free = xstrndup(s, n);\n+\t\tnew_branch = new_branch_to_free = xmemdupz(s, path + n - s);\n \t} else if (opts.orphan) {\n \t\t; /* no-op */\n \t} else if (opts.detach) {\ndiff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh\nindex 280d2e2c07..7e2811fa77 100755\n--- a/t/t2400-worktree-add.sh\n+++ b/t/t2400-worktree-add.sh\n@@ -298,6 +298,11 @@ test_expect_success '\"add\" with omitted' '\n \ttest_cmp_rev HEAD bat\n '\n \n+test_expect_success '\"add\" with trailing slash and omitted' '\n+\tgit worktree add waffle/bit/ &&\n+\ttest_cmp_rev HEAD bit\n+'\n+\n test_expect_success '\"add\" checks out existing branch of dwimd name' '\n \tgit branch dwim HEAD~1 &&\n \tgit worktree add dwim &&\n@@ -388,6 +393,14 @@ test_expect_success '\"add --orphan (no -b)\"' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success '\"add --orphan with trailing slash (no -b)\"' '\n+\ttest_when_finished \"git worktree remove -f -f neworphan\" &&\n+\tgit worktree add --orphan ./neworphan/ &&\n+\techo refs/heads/neworphan >expected &&\n+\tgit -C neworphan symbolic-ref HEAD >actual &&\n+\ttest_cmp expected actual\n+'\n+\n test_expect_success '\"add --orphan --quiet\"' '\n \ttest_when_finished \"git worktree remove -f -f orphandir\" &&\n \tgit worktree add --quiet --orphan -b neworphan orphandir 2>log.actual &&\n-- \n2.55.0\n\n"},{"id":"551213","messageId":"20260825180350.2099-2-l.s.r@web.de","threadId":"66218","inReplyTo":"20260825180350.2099-1-l.s.r@web.de","subject":"[PATCH 1/4] worktree add: don't read out of bounds in worktree_basename()","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2026-08-25T18:03:47Z","receivedAt":"2026-08-25T18:04:05Z","isPatch":true,"body":"When we search for the start of the basename and `len` is zero, `name`\nends up being `path` - 1, out of bounds. Avoid that by checking before\ndecrementing.\n\nFixes https://github.com/git-for-windows/git/issues/6346.\n\nOriginal-patch-by: Matthias Aßhauer \nSigned-off-by: René Scharfe \n---\n builtin/worktree.c | 8 +++-----\n 1 file changed, 3 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex 654d27c3e1..a770dd5ead 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -303,11 +303,9 @@ static const char *worktree_basename(const char *path, int *olen)\n \twhile (len && is_dir_sep(path[len - 1]))\n \t\tlen--;\n \n-\tfor (name = path + len - 1; name > path; name--)\n-\t\tif (is_dir_sep(*name)) {\n-\t\t\tname++;\n-\t\t\tbreak;\n-\t\t}\n+\tname = path + len;\n+\twhile (name > path && !is_dir_sep(name[-1]))\n+\t\tname--;\n \n \t*olen = len;\n \treturn name;\n-- \n2.55.0\n\n"},{"id":"551214","messageId":"20260825180350.2099-3-l.s.r@web.de","threadId":"66218","inReplyTo":"20260825180350.2099-1-l.s.r@web.de","subject":"[PATCH 2/4] worktree add: reject separator-only path","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2026-08-25T18:03:48Z","receivedAt":"2026-08-25T18:04:06Z","isPatch":true,"body":"worktree_basename() extracts an empty basename from a path consisting\nonly of zero or more path separators. We can't use that as a worktree\nname. Properly report such a path as invalid instead of triggering a\nBUG that asks the user what just happened.\n\nOriginal-patch-by: Matthias Aßhauer \nSigned-off-by: René Scharfe \n---\n builtin/worktree.c | 2 ++\n t/t2400-worktree-add.sh | 4 ++++\n 2 files changed, 6 insertions(+)\n\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex a770dd5ead..a53e815cc9 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -491,6 +491,8 @@ static int add_worktree(const char *path, const char *refname,\n \n \tname = worktree_basename(path, &len);\n \tstrbuf_add(&sb, name, path + len - name);\n+\tif (!sb.len)\n+\t\tdie(_(\"invalid path '%s'\"), path);\n \tsanitize_refname_component(sb.buf, &sb_name);\n \tif (!sb_name.len)\n \t\tBUG(\"How come '%s' becomes empty after sanitization?\", sb.buf);\ndiff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh\nindex 87b926728a..280d2e2c07 100755\n--- a/t/t2400-worktree-add.sh\n+++ b/t/t2400-worktree-add.sh\n@@ -46,6 +46,10 @@ test_expect_success '\"add\" refuses to checkout locked branch' '\n \ttest_path_is_missing .git/worktrees/zere\n '\n \n+test_expect_success '\"add\" rejects an empty path' '\n+\ttest_must_fail git worktree add \"\" HEAD\n+'\n+\n test_expect_success 'checking out paths not complaining about linked checkouts' '\n \t(\n \tcd existing_empty &&\n-- \n2.55.0\n\n"},{"id":"551234","messageId":"xmqq33w2m186.fsf@gitster.g","threadId":"66218","inReplyTo":"20260825180350.2099-4-l.s.r@web.de","subject":"Re: [PATCH 3/4] worktree add: trim slashes when deriving branch name from path","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-08-25T19:47:21Z","receivedAt":"2026-08-25T19:47:23Z","isPatch":true,"body":"René Scharfe writes:\n\n> worktree_basename() sets `n` to the length of `path` without trailing\n> path separators, not to the length of the basename. This matters when\n> deriving a branch name from a path with more than one component. E.g.:\n>\n> path: /new/worktree/\n> s: ^\n> n: |-----------|\n>\n> So here xstrndup(s, n) copies up to 13 characters from \"worktree/\",\n> effectively to the end of the string, including the trailing dash.\n>\n> Path separators are not allowed at the end of branch names, so strip\n> them off by calculating the basename length and extracting just that\n> part.\n>\n> Signed-off-by: René Scharfe \n> ---\n> builtin/worktree.c | 4 ++--\n> t/t2400-worktree-add.sh | 13 +++++++++++++\n> 2 files changed, 15 insertions(+), 2 deletions(-)\n\nHmph, so am I correct to understand that the symptom observable by\nend-users of this is that we used to attempt creating \"bit/\" branch\nwhen they request\n\n $ git worktree add waffle/bit/\n\nand it wouldn't have worked until they said\n\n $ git worktree add waffle/bit\n\ninstead? Not allowing a trailing slash when naming a directory is\nnasty (it is a good practice to explicitly give a trailing slash\nwhen naming a directory to avoid confusion), and it is a good fix.\n\nIt also should work fine with waffle/bit/// even though there is no\nstrong reason to allow it ;-).\n\n\n> diff --git a/builtin/worktree.c b/builtin/worktree.c\n> index a53e815cc9..01c245778e 100644\n> --- a/builtin/worktree.c\n> +++ b/builtin/worktree.c\n> @@ -769,7 +769,7 @@ static char *dwim_branch(const char *path, char **new_branch)\n> \tint n;\n> \tint branch_exists;\n> \tconst char *s = worktree_basename(path, &n);\n> -\tchar *branchname = xstrndup(s, n);\n> +\tchar *branchname = xmemdupz(s, path + n - s);\n> \tstruct strbuf ref = STRBUF_INIT;\n> \n> \tbranch_exists = !check_branch_ref(the_repository, &ref, branchname) &&\n> @@ -878,7 +878,7 @@ static int add(int ac, const char **av, const char *prefix,\n> \tif (opts.orphan && !new_branch) {\n> \t\tint n;\n> \t\tconst char *s = worktree_basename(path, &n);\n> -\t\tnew_branch = new_branch_to_free = xstrndup(s, n);\n> +\t\tnew_branch = new_branch_to_free = xmemdupz(s, path + n - s);\n> \t} else if (opts.orphan) {\n> \t\t; /* no-op */\n> \t} else if (opts.detach) {\n> diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh\n> index 280d2e2c07..7e2811fa77 100755\n> --- a/t/t2400-worktree-add.sh\n> +++ b/t/t2400-worktree-add.sh\n> @@ -298,6 +298,11 @@ test_expect_success '\"add\" with omitted' '\n> \ttest_cmp_rev HEAD bat\n> '\n> \n> +test_expect_success '\"add\" with trailing slash and omitted' '\n> +\tgit worktree add waffle/bit/ &&\n> +\ttest_cmp_rev HEAD bit\n> +'\n> +\n> test_expect_success '\"add\" checks out existing branch of dwimd name' '\n> \tgit branch dwim HEAD~1 &&\n> \tgit worktree add dwim &&\n> @@ -388,6 +393,14 @@ test_expect_success '\"add --orphan (no -b)\"' '\n> \ttest_cmp expected actual\n> '\n> \n> +test_expect_success '\"add --orphan with trailing slash (no -b)\"' '\n> +\ttest_when_finished \"git worktree remove -f -f neworphan\" &&\n> +\tgit worktree add --orphan ./neworphan/ &&\n> +\techo refs/heads/neworphan >expected &&\n> +\tgit -C neworphan symbolic-ref HEAD >actual &&\n> +\ttest_cmp expected actual\n> +'\n> +\n> test_expect_success '\"add --orphan --quiet\"' '\n> \ttest_when_finished \"git worktree remove -f -f orphandir\" &&\n> \tgit worktree add --quiet --orphan -b neworphan orphandir 2>log.actual &&\n"},{"id":"551235","messageId":"xmqqld9uklud.fsf@gitster.g","threadId":"66218","inReplyTo":"20260825180350.2099-5-l.s.r@web.de","subject":"Re: [PATCH 4/4] worktree add: let worktree_basename() return string copy","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-08-25T20:04:58Z","receivedAt":"2026-08-25T20:05:00Z","isPatch":true,"body":"René Scharfe writes:\n\n> worktree_basename() requires callers to do pointer arithmetic to get the\n> actual basename. Simplify them by doing the calculations in the\n> function and returning a copy of the basename directly.\n\nOK.\n\n> Remind programmers to free the result by renaming the function to\n> worktree_basename_dup(). Two already do; convert the remaining one from\n\nThis is a bit surprising, depending on what \"do\" refers to, as I\nread it to mean \"Two callers already free what is returned by the\nworktree_basename() function\", which cannot be the case (or they\nwould be segfaulting already). So I must have misunderstood this\nsentence. I count three callers of the function, so two do\nsomething while the other one that needs conversion does something\nelse.\n\n> resetting a shared strbuf to freeing the allocated string, which\n> requires the same number of lines, but no arithmetic. The added\n> allocation is negligible because it's small and there's only one per run\n> of \"git worktree add\".\n\nThis talks about the caller in builtin/worktree.c:add_worktree(),\nand it is indeed far easier to read with this patch applied, as\nthere is no need to copy out only the basename part, and we no\nlonger need to worry about chomping trailing directory separators.\n\n> @@ -766,10 +765,8 @@ static int dwim_orphan(const struct add_opts *opts, int opt_track, int remote)\n> \n> static char *dwim_branch(const char *path, char **new_branch)\n> {\n> -\tint n;\n> \tint branch_exists;\n> -\tconst char *s = worktree_basename(path, &n);\n> -\tchar *branchname = xmemdupz(s, path + n - s);\n> +\tchar *branchname = worktree_basename_dup(path);\n> \tstruct strbuf ref = STRBUF_INIT;\n\nAh, OK, so this is what you mean by \"two already do\". Not \"two\nalready free the result\", but \"two already make a copy before doing\nanything else anyway, so why not make worktree_basename_dup() give\nthem their own copies?\". Makes sense.\n\n> @@ -876,9 +873,7 @@ static int add(int ac, const char **av, const char *prefix,\n> \t}\n> \n> \tif (opts.orphan && !new_branch) {\n> -\t\tint n;\n> -\t\tconst char *s = worktree_basename(path, &n);\n> -\t\tnew_branch = new_branch_to_free = xmemdupz(s, path + n - s);\n> +\t\tnew_branch = new_branch_to_free = worktree_basename_dup(path);\n\nLikewise.\n\n\nSo going back to the confusing part of the log message,\n\n Remind ... to worktree_basename_dup(). Among the three callers\n of worktree_basename(), two immediately make copies of the\n returned string before using and freeing it, which makes for an\n easy conversion. Convert the other one from resetting ...\n\nor something like that, perhaps?\n\nThanks.\n"},{"id":"551259","messageId":"18e65a59-2d33-4f47-a5eb-ca5971cec482@web.de","threadId":"66218","inReplyTo":"xmqqld9uklud.fsf@gitster.g","subject":"Re: [PATCH 4/4] worktree add: let worktree_basename() return string copy","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2026-08-26T04:37:47Z","receivedAt":"2026-08-26T04:37:52Z","isPatch":true,"body":"On 8/25/26 10:04 PM, Junio C Hamano wrote:\n> René Scharfe writes:\n> \n>> worktree_basename() requires callers to do pointer arithmetic to get the\n>> actual basename. Simplify them by doing the calculations in the\n>> function and returning a copy of the basename directly.\n> \n> OK.\n> \n>> Remind programmers to free the result by renaming the function to\n>> worktree_basename_dup(). Two already do; convert the remaining one from\n> \n> This is a bit surprising, depending on what \"do\" refers to, as I\n> read it to mean \"Two callers already free what is returned by the\n> worktree_basename() function\", which cannot be the case (or they\n> would be segfaulting already). So I must have misunderstood this\n> sentence. I count three callers of the function, so two do\n> something while the other one that needs conversion does something\n> else.\n\nIt's confusing because I changed \"callers\" to \"programmers\" last\nminute and forgot to adjust the next sentence.\n\n>> resetting a shared strbuf to freeing the allocated string, which\n>> requires the same number of lines, but no arithmetic. The added\n>> allocation is negligible because it's small and there's only one per run\n>> of \"git worktree add\".\n\n> So going back to the confusing part of the log message,\n> \n> Remind ... to worktree_basename_dup(). Among the three callers\n> of worktree_basename(), two immediately make copies of the\n> returned string before using and freeing it, which makes for an\n> easy conversion. Convert the other one from resetting ...\n> \n> or something like that, perhaps?\n\nYes.\n\nRené\n\n"},{"id":"551286","messageId":"xmqqjypdj6g4.fsf@gitster.g","threadId":"66218","inReplyTo":"18e65a59-2d33-4f47-a5eb-ca5971cec482@web.de","subject":"Re: [PATCH 4/4] worktree add: let worktree_basename() return string copy","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-08-26T14:35:07Z","receivedAt":"2026-08-26T14:35:09Z","isPatch":true,"body":"René Scharfe writes:\n\n> On 8/25/26 10:04 PM, Junio C Hamano wrote:\n>> René Scharfe writes:\n>> \n>>> worktree_basename() requires callers to do pointer arithmetic to get the\n>>> actual basename. Simplify them by doing the calculations in the\n>>> function and returning a copy of the basename directly.\n>> \n>> OK.\n>> \n>>> Remind programmers to free the result by renaming the function to\n>>> worktree_basename_dup(). Two already do; convert the remaining one from\n>> \n>> This is a bit surprising, depending on what \"do\" refers to, as I\n>> read it to mean \"Two callers already free what is returned by the\n>> worktree_basename() function\", which cannot be the case (or they\n>> would be segfaulting already). So I must have misunderstood this\n>> sentence. I count three callers of the function, so two do\n>> something while the other one that needs conversion does something\n>> else.\n>\n> It's confusing because I changed \"callers\" to \"programmers\" last\n> minute and forgot to adjust the next sentence.\n>\n>>> resetting a shared strbuf to freeing the allocated string, which\n>>> requires the same number of lines, but no arithmetic. The added\n>>> allocation is negligible because it's small and there's only one per run\n>>> of \"git worktree add\".\n>\n>> So going back to the confusing part of the log message,\n>> \n>> Remind ... to worktree_basename_dup(). Among the three callers\n>> of worktree_basename(), two immediately make copies of the\n>> returned string before using and freeing it, which makes for an\n>> easy conversion. Convert the other one from resetting ...\n>> \n>> or something like that, perhaps?\n>\n> Yes.\n\nThanks. We do not know if other parts of the series gets more\nserious reviews that necessitates an updated version, so in the\nmeantime I'll reword what I have locally.\n\n"},{"id":"551592","messageId":"xmqqfqzuw23a.fsf@gitster.g","threadId":"66218","inReplyTo":"xmqqjypdj6g4.fsf@gitster.g","subject":"Re: [PATCH 4/4] worktree add: let worktree_basename() return string copy","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-08-31T18:56:41Z","receivedAt":"2026-08-31T18:56:44Z","isPatch":true,"body":"Junio C Hamano writes:\n\n>>> So going back to the confusing part of the log message,\n>>> \n>>> Remind ... to worktree_basename_dup(). Among the three callers\n>>> of worktree_basename(), two immediately make copies of the\n>>> returned string before using and freeing it, which makes for an\n>>> easy conversion. Convert the other one from resetting ...\n>>> \n>>> or something like that, perhaps?\n>>\n>> Yes.\n>\n> Thanks. We do not know if other parts of the series gets more\n> serious reviews that necessitates an updated version, so in the\n> meantime I'll reword what I have locally.\n\nAnd nothing happened since then. As the topic was in a good shape\nexcept for the confusing part of the log, which we amended in my\ntree already, let's mark the topic for 'next'.\n"}]}