From: Phillip Wood Date: Thu, 30 Nov 2023 15:22:32 GMT Subject: Re: [PATCH 2/2] checkout: forbid "-B " from touching a branch used elsewhere Message-ID: In-Reply-To: Hi Junio On 27/11/2023 01:51, Junio C Hamano wrote: > Phillip Wood 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.