Re: [PATCH 1/2] shallow: free local object_array allocations
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Jan 6, 2026, 07:44 UTC
- Message-ID
- <aVy9ZveUOg3yum2X@pks.im>
- In-Reply-To
- <277c8616a9fc365b76b2f4ab458cd927834f9e0e.1765303880.git.gitgitgadget@gmail.com>
On Tue, Dec 09, 2025 at 06:11:19PM +0000, Samo Pogačnik via GitGitGadget wrote:
Show 5 quoted lines
> From: =?UTF-8?q?Samo=20Poga=C4=8Dnik?= <samo_pogacnik@t-2.net> > > The local object_array 'stack' in get_shallow_commits() function > does not free its dynamic elements before the function returns. > As a result elements remain allocated and their reference forgotten.
I think the elements themselves are actually fine. We have the following loop:
while (commit || i < heads->nr || stack.nr) {So while the stack still has entries, we'll keep on iteration. Furthermore, there is no `break` or early return in the loop, so we can sure that we actually pop every single element from the array.
That being said, what we _don't_ do is to free the array itself. So I'm mostly splitting hairs with how the commit message is phrased, the change looks correct to me.
What I'm wondering though is why we never hit this memory leak in our test suite. I guess the reason is simply that we ain't got enough test coverage around shallow clones. Have you seen this leak in the wild? And if so, can we add a test case that surfaces it?
Thanks!
Patrick