Re: [PATCH 15/20] remote: fix leaking config strings
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Aug 22, 2024, 16:04 UTC
- Message-ID
- <xmqqcym0tuhb.fsf@gitster.g>
- In-Reply-To
- <Zsb0nspisMs1Csl-@tanuki>
Patrick Steinhardt <ps@pks.im> writes:
Show 19 quoted lines
> On Wed, Aug 21, 2024 at 10:58:55AM -0700, Junio C Hamano wrote: >> Patrick Steinhardt <ps@pks.im> 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.