From: Patrick Steinhardt <ps@pks.im>
Date: Fri, 10 May 2024 16:13:48 GMT
Subject: Re: [PATCH 00/13] reftable: prepare for re-seekable iterators
Message-ID: <Zj5HvF-Uq0uWranj@framework>
In-Reply-To: <xmqq1q697jzr.fsf@gitster.g>

On Fri, May 10, 2024 at 08:40:56AM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > The below diff would be needed on top of what you currently have in
> > `seen`. In any case though, I can also resend this topic with
> > ps/reftable-write-options pulled in as a dependency. Please let me know
> > your preference.
> 
> Is it "needed", in the sense that "without the fix what you posted
> is broken in such and such ways", or is it "I think it is niceR to
> have it on stack because this one instance does not have to be on
> heap"?  To me, they look equivalent and I have no problems with the
> "nicer" variant, but your "needed" makes me wonder if I am missing
> some correctness invariants I am violating without realizing.

It's needed in the sense that your version leaks memory -- the `ti`
pointer is never free'd. Other than that they are equivalent indeed.

Patrick