Re: [PATCH] builtin/history: unuse the commit buffer after use

[Kaartic Sivaraam <kaartic.sivaraam@gmail.com>]

Hi Patrick,

On 15/06/26 15:18, Patrick Steinhardt wrote:
> Huh, curious. That seems to hint that we're missing test coverage for
> this specific scenario, as our test suite doesn't detect this leak.
>

Indeed. The tricky thing is (as mentioned in another thread), this is 
happening only when we get a commit not cached in the commit slab. Once 
we get an idea on how certain commits get cached in the commit slab 
while others don't, we can write a test case that would catch this leak.

> 
> So this doesn't really read specific at all, and I would have expected
> us to hit this leak. Puzzling.
>

Yeah. My bad with the commit message. The leak is not happening always. 
That is a reason we may not have caught this in the test suite.

>> I must mention that I also noticed the following comment in `commit_tree_ext`:
>>
>> »       /* We retain authorship of the original commit. */
>> »       original_message = repo_logmsg_reencode(repo, commit_with_message, NULL, NULL);
>>
>> ... but I'm not quite sure why we don't unuse the buffer after its purpose is
>> done. Kindly englighten me in case I missed something.
> 
> Did you maybe confuse "authorship" with "ownership" while reading the
> comment? The comment only mentions that we retain the original "Author"
> commit metadata, it doesn't refer to ownership of the underlying
> objects.
>

Got it. Thank you for the correction!

--
Sivaraam

PS: Sorry about the delay in response. Was stuck with some personal work.