Re: [PATCH 2/2] checkout: forbid "-B <branch>" from touching a branch used elsewhere
- From
- Phillip 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.