On Thu, Aug 08, 2024 at 03:04:33PM +0200, Patrick Steinhardt wrote: > When we have a `url.*.insteadOf` configuration, then we end up aliasing > URLs when populating remotes. One place where this happens is in > `alias_all_urls()`, where we loop through all remotes and then alias > each of their URLs. The actual aliasing logic is then contained in > `alias_url()`, which returns an allocated string that contains the new > URL. This URL replaces the old URL that we have in the strvec that > contanis all remote URLs. > > We replace the remote URLs via `strvec_replace()`, which does not hand > over ownership of the new string to the vector. Still, we didn't free > the aliased URL and thus have a memory leak here. Fix it by freeing the > aliased string. Thanks for the detailed explanation here. > Signed-off-by: Patrick Steinhardt <ps@xxxxxx> > --- > remote.c | 2 ++ > t/t0210-trace2-normal.sh | 2 +- > 2 files changed, 3 insertions(+), 1 deletion(-) > > diff --git a/remote.c b/remote.c > index f43cf5e7a4..3b898edd23 100644 > --- a/remote.c > +++ b/remote.c > @@ -499,6 +499,7 @@ static void alias_all_urls(struct remote_state *remote_state) > if (alias) > strvec_replace(&remote_state->remotes[i]->pushurl, > j, alias); > + free(alias); > } > add_pushurl_aliases = remote_state->remotes[i]->pushurl.nr == 0; > for (j = 0; j < remote_state->remotes[i]->url.nr; j++) { > @@ -512,6 +513,7 @@ static void alias_all_urls(struct remote_state *remote_state) > if (alias) > strvec_replace(&remote_state->remotes[i]->url, > j, alias); > + free(alias); > } > } > } These both make sense to me, since alias_url() allocates the string it returns via xstrfmt(), so having the caller free it makes sense. I was wondering if there was a nice way to neaten up these two call paths that both call alias_url(), check for NULL, call strbuf_replace(), and then free the result. But I think the result here would be pretty awkward from my attempts at it, so I think this patch looks good as-is. Thanks, Taylor