Subject: Re: [PATCH 15/20] remote: fix leaking config strings

From: Junio C Hamano <[email-masked]>
Date: Thu, 22 Aug 2024 09:04:00 -0700

Patrick Steinhardt <[email-masked]> writes:

> On Wed, Aug 21, 2024 at 10:58:55AM -0700, Junio C Hamano wrote:
>> Patrick Steinhardt <[email-masked]> writes:
>> 
>> > @@ -2802,6 +2809,7 @@ void remote_state_clear(struct remote_state *remote_state)
>> >  	for (i = 0; i < remote_state->remotes_nr; i++)
>> >  		remote_clear(remote_state->remotes[i]);
>> >  	FREE_AND_NULL(remote_state->remotes);
>> > +	FREE_AND_NULL(remote_state->pushremote_name);
>> >  	remote_state->remotes_alloc = 0;
>> >  	remote_state->remotes_nr = 0;
>> 
>> As remote_state has two extra structures embedded in it, I wonder if
>> we should be clearing them in this function, but possibly it is
>> cleared elsewhere or perhaps in a later series?
>
> It is not yet part of any subsequent patch series, mostly because I
> didn't happen to stumble over such leaks yet. Both of the rewrites very
> much are leaky though, and would be hit when we use "insteadOf" or
> "pushInsteadOf" configs.

Yes, I was wondering if our test coverage for the feature is
lacking.  In a sense, leaks from these insteadOf configuration
variables are tiny and uninteresting, compared to how much it
bothers me to find us not testing these often used features.