git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2 19/20] transport: fix leaking arguments when fetching from bundle

From
Junio C Hamano <gitster@pobox.com>
Date
Aug 22, 2024, 18:21 UTC
Message-ID
<xmqqmsl4qv02.fsf@gitster.g>
In-Reply-To
<4c5740afe43c3bb619a9cba0c1634097c27ed33f.1724315484.git.ps@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
Show 63 quoted lines
> In `fetch_refs_from_bundle()` we assemble a vector of arguments to pass
> to `unbundle()`, but never free it. And in theory we wouldn't have to
> because `unbundle()` already knows to free the vector for us. But it
> fails to do so when it exits early due to `verify_bundle()` failing.
>
> The calling convention that the arguments are freed by the callee and
> not the caller feels somewhat weird. Refactor the code such that it is
> instead the responsibility of the caller to free the vector, adapting
> the only two callsites where we pass extra arguments. This also fixes
> the memory leak.
>
> This memory leak gets hit in t5510, but fixing it isn't sufficient to
> make the whole test suite pass.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  builtin/bundle.c | 2 ++
>  bundle.c         | 4 +---
>  transport.c      | 2 ++
>  3 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/builtin/bundle.c b/builtin/bundle.c
> index d5d41a8f672..df97f399019 100644
> --- a/builtin/bundle.c
> +++ b/builtin/bundle.c
> @@ -220,7 +220,9 @@ static int cmd_bundle_unbundle(int argc, const char **argv, const char *prefix)
>  			 &extra_index_pack_args, 0) ||
>  		list_bundle_refs(&header, argc, argv);
>  	bundle_header_release(&header);
> +
>  cleanup:
> +	strvec_clear(&extra_index_pack_args);
>  	free(bundle_file);
>  	return ret;
>  }
> diff --git a/bundle.c b/bundle.c
> index ce164c37bc8..0f6c7a71ef1 100644
> --- a/bundle.c
> +++ b/bundle.c
> @@ -639,10 +639,8 @@ int unbundle(struct repository *r, struct bundle_header *header,
>  	if (flags & VERIFY_BUNDLE_FSCK)
>  		strvec_push(&ip.args, "--fsck-objects");
>  
> -	if (extra_index_pack_args) {
> +	if (extra_index_pack_args)
>  		strvec_pushv(&ip.args, extra_index_pack_args->v);
> -		strvec_clear(extra_index_pack_args);
> -	}
>  
>  	ip.in = bundle_fd;
>  	ip.no_stdout = 1;
> diff --git a/transport.c b/transport.c
> index f0672fdc505..da639d3bff0 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -189,6 +189,8 @@ static int fetch_refs_from_bundle(struct transport *transport,
>  		       &extra_index_pack_args,
>  		       fetch_pack_fsck_objects() ? VERIFY_BUNDLE_FSCK : 0);
>  	transport->hash_algo = data->header.hash_algo;
> +
> +	strvec_clear(&extra_index_pack_args);
>  	return ret;
>  }
Looks much better.  Will queue.  Thanks.
Previous: Patrick SteinhardtNext: Patrick Steinhardt
Message 58 of 60 in “Memory leak fixes (pt.5)”
  1. 00/20 Memory leak fixes (pt.5)Patrick Steinhardt, Aug 20, 2024
  2. 01/20 mailinfo: fix leaking header dataPatrick Steinhardt, Aug 20, 2024
  3. Junio C HamanoAug 20, 2024
  4. 02/20 convert: fix leaks when resetting attributesPatrick Steinhardt, Aug 20, 2024
  5. Junio C HamanoAug 20, 2024
  6. 03/20 pretty: fix memory leaks when parsing pretty formatsPatrick Steinhardt, Aug 20, 2024
  7. Junio C HamanoAug 20, 2024
  8. Patrick SteinhardtAug 22, 2024
  9. 04/20 pretty: fix leaking key/value separator bufferPatrick Steinhardt, Aug 20, 2024
  10. 05/20 builtin/merge-tree: fix leaking `-X` strategy optionsPatrick Steinhardt, Aug 20, 2024
  11. Junio C HamanoAug 20, 2024
  12. 06/20 builtin/upload-archive: fix leaking args passed to `write_archive()`Patrick Steinhardt, Aug 20, 2024
  13. 07/20 builtin/archive: fix leaking `OPT_FILENAME()` valuePatrick Steinhardt, Aug 20, 2024
  14. 08/20 midx-write: fix leaking hashfile on error casesPatrick Steinhardt, Aug 20, 2024
  15. Junio C HamanoAug 20, 2024
  16. Patrick SteinhardtAug 22, 2024
  17. 09/20 builtin/repack: fix leaks when computing packs to repackPatrick Steinhardt, Aug 20, 2024
  18. 10/20 t/helper: fix leaking multi-pack-indices in "read-midx"Patrick Steinhardt, Aug 20, 2024
  19. 11/20 transport: fix leaking OID arrays in git:// transport dataPatrick Steinhardt, Aug 20, 2024
  20. 12/20 builtin/send-pack: fix leaking refspecsPatrick Steinhardt, Aug 20, 2024
  21. 13/20 sideband: fix leaks when configuring sideband colorsPatrick Steinhardt, Aug 20, 2024
  22. Junio C HamanoAug 20, 2024
  23. Patrick SteinhardtAug 22, 2024
  24. 14/20 builtin/fetch-pack: fix leaking refsPatrick Steinhardt, Aug 20, 2024
  25. Junio C HamanoAug 21, 2024
  26. 15/20 remote: fix leaking config stringsPatrick Steinhardt, Aug 20, 2024
  27. Junio C HamanoAug 21, 2024
  28. Patrick SteinhardtAug 22, 2024
  29. Junio C HamanoAug 22, 2024
  30. 16/20 remote: fix leaks when matching refspecsPatrick Steinhardt, Aug 20, 2024
  31. 17/20 remote: fix leaking peer ref when expanding refmapPatrick Steinhardt, Aug 20, 2024
  32. 18/20 builtin/fetch: fix leaking transaction with `--atomic`Patrick Steinhardt, Aug 20, 2024
  33. 19/20 transport: fix leaking arguments when fetching from bundlePatrick Steinhardt, Aug 20, 2024
  34. Junio C HamanoAug 21, 2024
  35. Patrick SteinhardtAug 22, 2024
  36. Junio C HamanoAug 22, 2024
  37. 20/20 transport: fix leaking negotiation tipsPatrick Steinhardt, Aug 20, 2024
  38. 00/20 Memory leak fixes (pt.5)Patrick Steinhardt, Aug 22, 2024
  39. 01/20 mailinfo: fix leaking header dataPatrick Steinhardt, Aug 22, 2024
  40. 02/20 convert: fix leaks when resetting attributesPatrick Steinhardt, Aug 22, 2024
  41. 03/20 pretty: fix memory leaks when parsing pretty formatsPatrick Steinhardt, Aug 22, 2024
  42. 04/20 pretty: fix leaking key/value separator bufferPatrick Steinhardt, Aug 22, 2024
  43. 05/20 builtin/merge-tree: fix leaking `-X` strategy optionsPatrick Steinhardt, Aug 22, 2024
  44. 06/20 builtin/upload-archive: fix leaking args passed to `write_archive()`Patrick Steinhardt, Aug 22, 2024
  45. 07/20 builtin/archive: fix leaking `OPT_FILENAME()` valuePatrick Steinhardt, Aug 22, 2024
  46. 08/20 midx-write: fix leaking hashfile on error casesPatrick Steinhardt, Aug 22, 2024
  47. 09/20 builtin/repack: fix leaks when computing packs to repackPatrick Steinhardt, Aug 22, 2024
  48. 10/20 t/helper: fix leaking multi-pack-indices in "read-midx"Patrick Steinhardt, Aug 22, 2024
  49. 11/20 transport: fix leaking OID arrays in git:// transport dataPatrick Steinhardt, Aug 22, 2024
  50. 12/20 builtin/send-pack: fix leaking refspecsPatrick Steinhardt, Aug 22, 2024
  51. 13/20 sideband: fix leaks when configuring sideband colorsPatrick Steinhardt, Aug 22, 2024
  52. 14/20 builtin/fetch-pack: fix leaking refsPatrick Steinhardt, Aug 22, 2024
  53. 15/20 remote: fix leaking config stringsPatrick Steinhardt, Aug 22, 2024
  54. 16/20 remote: fix leaks when matching refspecsPatrick Steinhardt, Aug 22, 2024
  55. 17/20 remote: fix leaking peer ref when expanding refmapPatrick Steinhardt, Aug 22, 2024
  56. 18/20 builtin/fetch: fix leaking transaction with `--atomic`Patrick Steinhardt, Aug 22, 2024
  57. 19/20 transport: fix leaking arguments when fetching from bundlePatrick Steinhardt, Aug 22, 2024
  58. Junio C HamanoAug 22, 2024
  59. 20/20 transport: fix leaking negotiation tipsPatrick Steinhardt, Aug 22, 2024
  60. Junio C HamanoAug 22, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.