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, this one is my fault. When I replaced the open-coded replacement in 8e804415fd (remote: use strvecs to store remote url/pushurl, 2024-06-14), for some reason I thought that strvec_replace() would take ownership of the pointer. We could make a "_nodup()" variant, but it is probably not worth the extra API complexity. Curiously, these are the only calls for strvec_replace(). You added it in 11ce77b5cc (strvec: add functions to replace and remove strings, 2024-05-27) but I don't see them used in any iteration of that patch series. So yet another option is to change the semantics of strvec_replace(), but I think that is an even worse idea. ;) -Peff
