git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 4/4] worktree add: let worktree_basename() return string copy

From
Junio C Hamano <gitster@pobox.com>
Date
Aug 26, 2026, 14:35 UTC
Message-ID
<xmqqjypdj6g4.fsf@gitster.g>
In-Reply-To
<18e65a59-2d33-4f47-a5eb-ca5971cec482@web.de>
René Scharfe <l.s.r@web.de> writes:
Show 38 quoted lines
> On 8/25/26 10:04 PM, Junio C Hamano wrote:
>> René Scharfe <l.s.r@web.de> writes:
>> 
>>> worktree_basename() requires callers to do pointer arithmetic to get the
>>> actual basename.  Simplify them by doing the calculations in the
>>> function and returning a copy of the basename directly.
>> 
>> OK.
>> 
>>> Remind programmers to free the result by renaming the function to
>>> worktree_basename_dup().  Two already do; convert the remaining one from
>> 
>> This is a bit surprising, depending on what "do" refers to, as I
>> read it to mean "Two callers already free what is returned by the
>> worktree_basename() function", which cannot be the case (or they
>> would be segfaulting already).  So I must have misunderstood this
>> sentence.  I count three callers of the function, so two do
>> something while the other one that needs conversion does something
>> else.
>
> It's confusing because I changed "callers" to "programmers" last
> minute and forgot to adjust the next sentence.
>
>>> resetting a shared strbuf to freeing the allocated string, which
>>> requires the same number of lines, but no arithmetic.  The added
>>> allocation is negligible because it's small and there's only one per run
>>> of "git worktree add".
>
>> So going back to the confusing part of the log message,
>> 
>>     Remind ... to worktree_basename_dup().  Among the three callers
>>     of worktree_basename(), two immediately make copies of the
>>     returned string before using and freeing it, which makes for an
>>     easy conversion.  Convert the other one from resetting ...
>> 
>> or something like that, perhaps?
>
> Yes.

Thanks. We do not know if other parts of the series gets more serious reviews that necessitates an updated version, so in the meantime I'll reword what I have locally.

Previous: René ScharfeNext: Junio C Hamano
Message 5 of 10 in “worktree add: worktree_basename() fixes”
  1. 0/4 worktree add: worktree_basename() fixesRené Scharfe, Aug 25, 2026
  2. 4/4 worktree add: let worktree_basename() return string copyRené Scharfe, Aug 25, 2026
  3. Junio C HamanoAug 25, 2026
  4. René ScharfeAug 26, 2026
  5. Junio C HamanoAug 26, 2026
  6. Junio C HamanoAug 31, 2026
  7. 3/4 worktree add: trim slashes when deriving branch name from pathRené Scharfe, Aug 25, 2026
  8. Junio C HamanoAug 25, 2026
  9. 1/4 worktree add: don't read out of bounds in worktree_basename()René Scharfe, Aug 25, 2026
  10. 2/4 worktree add: reject separator-only pathRené Scharfe, Aug 25, 2026

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.