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

[Phillip Wood <phillip.wood123@gmail.com>]

Hi Junio

On 27/11/2023 01:51, Junio C Hamano wrote:
> 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

> 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.