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

Re: [PATCH 2/2] checkout: forbid "-B <branch>" from touching a branch used elsewhere

From
PWPhillip Wood <phillip.wood123@gmail.com>
Date
Nov 30, 2023, 15:22 UTC
Message-ID
<b3532261-3cf4-4666-9cbd-4ce668cd2e49@gmail.com>
In-Reply-To
<xmqqwmu42ccb.fsf@gitster.g>
Hi Junio
On 27/11/2023 01:51, Junio C Hamano wrote:
Show 16 quoted lines
> Phillip Wood <phillip.wood123@gmail.com> writes:
> 
>> At the moment this is academic as neither of the test scripts changed
>> by this patch are leak free and so I don't think we need to worry
>> about it but it raises an interesting question about how we should
>> handle memory leaks when dying. Leaving the leak when dying means that
>> a test script that tests an expected failure will never be leak free
>> but using UNLEAK() would mean we miss a leak being introduced in the
>> successful case should the call to "free()" ever be removed.
> 
> Is there a leak here?  The piece of memory is pointed at by an on-stack
> variable full_ref when leak sanitizer starts scanning the heap and
> the stack just before the process exits due to die, so I do not see
> a reason to worry about this particular variable over all the other
> on stack variables we accumulated before the control reached this
> point of the code.

Oh, good point. I was thinking "we exit without calling free() so it is leaked" but as you say the leak checker (thankfully) does not consider it a leak as there is still a reference to the allocation on the stack.

Sorry for the noise
Phillip
Show 10 quoted lines
> Are you worried about optimizing compilers that behave more cleverly
> than their own good to somehow lose the on-stack reference to
> full_ref while calling die_if_switching_to_a_branch_in_use()?  We
> might need to squelch them with UNLEAK() but that does not mean we
> have to remove the free() we see above, and I suspect a more
> productive use of our time to solve that issue is ensure that our
> leak-sanitizing build will not triger such an unwanted optimization
> anyway.
> 
> Thanks.
Previous: Jeff KingNext: Willem Verstraeten
Message 10 of 17 in “git checkout -B <branch> lets you checkout a branch that is already checked out in another worktree Inbox”
  1. Willem VerstraetenNov 22, 2023
  2. Junio C HamanoNov 23, 2023
  3. Junio C HamanoNov 23, 2023
  4. 2/2 checkout: forbid "-B <branch>" from touching a branch used elsewhereJunio C Hamano, Nov 23, 2023
  5. Phillip WoodNov 23, 2023
  6. Eric SunshineNov 23, 2023
  7. Junio C HamanoNov 24, 2023
  8. Junio C HamanoNov 27, 2023
  9. Jeff KingNov 27, 2023
  10. Phillip WoodNov 30, 2023
  11. Willem VerstraetenDec 4, 2023
  12. Eric SunshineDec 4, 2023
  13. Junio C HamanoDec 8, 2023
  14. Willem VerstraetenJan 30, 2024
  15. Junio C HamanoJan 30, 2024
  16. Andy KoppeNov 23, 2023
  17. Willem VerstraetenNov 23, 2023

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.