| Message ID | cover.1722933642.git.ps@pks.im (mailing list archive) |
|---|---|
| Headers | show |
| Series | Memory leak fixes (pt.4) | expand |
On Tue Aug 6, 2024 at 6:59 PM AEST, Patrick Steinhardt wrote: > Hi, > > the third set of memory leak fixes was merged to `next`, so this is the > next part of more or less random memory leak fixes all over the place. > With this series, we're at ~155 leaking test suites. Naturally, I've > already got v5 in the pipeline, which brings us down to ~120. > > The series is built on top of 406f326d27 (The second batch, 2024-08-01) > with ps/leakfixes-part-3 at f30bfafcd4 (commit-reach: fix trivial memory > leak when computing reachability, 2024-08-01) merged into it. > > Thanks! > > Patrick Thanks Patrick, most of these fixes make sense to me! I appreciate that even the minor changes are accompanied by context. Cheers, James
Patrick Steinhardt <ps@pks.im> writes: > The series is built on top of 406f326d27 (The second batch, 2024-08-01) > with ps/leakfixes-part-3 at f30bfafcd4 (commit-reach: fix trivial memory > leak when computing reachability, 2024-08-01) merged into it. A quick question. Is it on your radar that transport_get() leaks the helper name when "foo::bar" is given as a remote? https://github.com/git/git/actions/runs/10274435719/job/28431161208#step:5:893 If not, I'll handle it separately, whose fix should look something like the attached. Thanks. transport.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git c/transport.c w/transport.c index 12cc5b4d96..13bf8183b7 100644 --- c/transport.c +++ w/transport.c @@ -1115,6 +1115,7 @@ static struct transport_vtable builtin_smart_vtable = { struct transport *transport_get(struct remote *remote, const char *url) { const char *helper; + char *helper_to_free = NULL; const char *p; struct transport *ret = xcalloc(1, sizeof(*ret)); @@ -1139,10 +1140,11 @@ struct transport *transport_get(struct remote *remote, const char *url) while (is_urlschemechar(p == url, *p)) p++; if (starts_with(p, "::")) - helper = xstrndup(url, p - url); + helper_to_free = helper = xstrndup(url, p - url); if (helper) { transport_helper_init(ret, helper); + free(helper_to_free); } else if (starts_with(url, "rsync:")) { die(_("git-over-rsync is no longer supported")); } else if (url_is_local_not_ssh(url) && is_file(url) && is_bundle(url, 1)) {
On Wed, Aug 07, 2024 at 09:59:39AM -0700, Junio C Hamano wrote: > Patrick Steinhardt <ps@pks.im> writes: > > > The series is built on top of 406f326d27 (The second batch, 2024-08-01) > > with ps/leakfixes-part-3 at f30bfafcd4 (commit-reach: fix trivial memory > > leak when computing reachability, 2024-08-01) merged into it. > > A quick question. Is it on your radar that transport_get() leaks > the helper name when "foo::bar" is given as a remote? > > https://github.com/git/git/actions/runs/10274435719/job/28431161208#step:5:893 > > If not, I'll handle it separately, whose fix should look something > like the attached. > > Thanks. Yeah, it's in part 5 [1], 97613b9cb9 (transport-helper: fix leaking helper name, 2024-05-27). Feel free to handle it separately though, I'll wait for part 4 to land first anyway, which likely takes a couple of days. Patrick [1]: https://gitlab.com/gitlab-org/git/-/tree/pks-leak-fixes-pt5
Patrick Steinhardt <ps@pks.im> writes: > On Wed, Aug 07, 2024 at 09:59:39AM -0700, Junio C Hamano wrote: >> Patrick Steinhardt <ps@pks.im> writes: >> >> > The series is built on top of 406f326d27 (The second batch, 2024-08-01) >> > with ps/leakfixes-part-3 at f30bfafcd4 (commit-reach: fix trivial memory >> > leak when computing reachability, 2024-08-01) merged into it. >> >> A quick question. Is it on your radar that transport_get() leaks >> the helper name when "foo::bar" is given as a remote? >> >> https://github.com/git/git/actions/runs/10274435719/job/28431161208#step:5:893 >> >> If not, I'll handle it separately, whose fix should look something >> like the attached. >> >> Thanks. > > Yeah, it's in part 5 [1], 97613b9cb9 (transport-helper: fix leaking > helper name, 2024-05-27). Feel free to handle it separately though, I'll > wait for part 4 to land first anyway, which likely takes a couple of > days. OK, will do, as this seems to break CI.
On Wed, Aug 07, 2024 at 07:27:32PM +1000, James Liu wrote: > On Tue Aug 6, 2024 at 6:59 PM AEST, Patrick Steinhardt wrote: > > Hi, > > > > the third set of memory leak fixes was merged to `next`, so this is the > > next part of more or less random memory leak fixes all over the place. > > With this series, we're at ~155 leaking test suites. Naturally, I've > > already got v5 in the pipeline, which brings us down to ~120. > > > > The series is built on top of 406f326d27 (The second batch, 2024-08-01) > > with ps/leakfixes-part-3 at f30bfafcd4 (commit-reach: fix trivial memory > > leak when computing reachability, 2024-08-01) merged into it. > > > > Thanks! > > > > Patrick > > Thanks Patrick, most of these fixes make sense to me! I appreciate that > even the minor changes are accompanied by context. Thanks for your review! Patrick
On Thu Aug 8, 2024 at 3:05 PM AEST, Patrick Steinhardt wrote: > On Wed, Aug 07, 2024 at 07:27:32PM +1000, James Liu wrote: > > On Tue Aug 6, 2024 at 6:59 PM AEST, Patrick Steinhardt wrote: > > > Hi, > > > > > > the third set of memory leak fixes was merged to `next`, so this is the > > > next part of more or less random memory leak fixes all over the place. > > > With this series, we're at ~155 leaking test suites. Naturally, I've > > > already got v5 in the pipeline, which brings us down to ~120. > > > > > > The series is built on top of 406f326d27 (The second batch, 2024-08-01) > > > with ps/leakfixes-part-3 at f30bfafcd4 (commit-reach: fix trivial memory > > > leak when computing reachability, 2024-08-01) merged into it. > > > > > > Thanks! > > > > > > Patrick > > > > Thanks Patrick, most of these fixes make sense to me! I appreciate that > > even the minor changes are accompanied by context. > > Thanks for your review! > > Patrick Thanks for responding to my questions! I don't have anything further to add. Cheers, James