Re: [PATCH v5 1/3] transport: add `transport_options_release()`

[Patrick Steinhardt <ps@pks.im>]

On Wed, Oct 28, 2026 at 04:18:11PM -0400, Karthik Nayak wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > On Wed, Oct 28, 2026 at 10:47:59AM +0100, Karthik Nayak wrote:
> >> The `git_transport_options` structure is used both in 'transport.c' and
> >> 'transport-helper.c'. But while the former specifically clears any
> >> associated data in the structure during the cleanup of the transport,
> >> the latter misses this.
> >>
> >> This means when using remote-helpers, data allocated for the structure
> >> leaks. Introduce a new `transport_options_release()`, which acts as a
> >
> > Nit: "a new transport_options_release" reads a bit weird. I'd either add
> > "function", or drop "a new".
> >
> 
> Fair enough, I'll drop the 'a new'.
> 
> >> common path to release memory allocated with the structure and use it in
> >> both files.
> >
> > Why didn't our CI detect this leak? Don't we have tests exercising this?
> >
> > The patch itself looks obviously good to me, thanks.
> >
> > Patrick
> 
> We don't have any tests which exercise this it seems. The tests in the
> next patch do, which is why it hit the bug!

That would have been great to point out in the commit message :) Thanks
for clarifying!

Patrick