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

Re: [PATCH 02/29] parse-options: fix leaks for users of OPT_FILENAME

From
Karthik Nayak <karthik.188@gmail.com>
Date
Jun 6, 2024, 10:00 UTC
Message-ID
<CAOLa=ZQj1UrNO+nOxhp2kaO+F1hHDE14FMYi74Eg8sMAgXz8OQ@mail.gmail.com>
In-Reply-To
<ecabbb74e19ea773fd23719bd7fd4937b29679e5.1717402439.git.ps@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
[snip]
Show 22 quoted lines
> diff --git a/builtin/archive.c b/builtin/archive.c
> index 15ee1ec7bb..f29c0ef6ad 100644
> --- a/builtin/archive.c
> +++ b/builtin/archive.c
> @@ -92,6 +92,7 @@ int cmd_archive(int argc, const char **argv, const char *prefix)
>  			N_("path to the remote git-upload-archive command")),
>  		OPT_END()
>  	};
> +	int ret;
>
>  	argc = parse_options(argc, argv, prefix, local_opts, NULL,
>  			     PARSE_OPT_KEEP_ALL);
> @@ -106,6 +107,8 @@ int cmd_archive(int argc, const char **argv, const char *prefix)
>
>  	setvbuf(stderr, NULL, _IOLBF, BUFSIZ);
>
> -	UNLEAK(output);
> -	return write_archive(argc, argv, prefix, the_repository, output, 0);
> +	ret = write_archive(argc, argv, prefix, the_repository, output, 0);
> +
> +	free(output);
> +	return ret;

Since output is passed to `write_archive`, we capture the return and free output. Makes sense.

[snip]
Show 23 quoted lines
> diff --git a/builtin/log.c b/builtin/log.c
> index 78a247d8a9..4e4b645a21 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -2021,7 +2021,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
>  	const char *rfc = NULL;
>  	int creation_factor = -1;
>  	const char *signature = git_version_string;
> -	const char *signature_file_arg = NULL;
> +	char *signature_file_arg = NULL;
>  	struct keep_callback_data keep_callback_data = {
>  		.cfg = &cfg,
>  		.revs = &rev,
> @@ -2559,6 +2559,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
>  	strbuf_release(&rdiff1);
>  	strbuf_release(&rdiff2);
>  	strbuf_release(&rdiff_title);
> +	free(description_file);
> +	free(signature_file_arg);
>  	free(to_free);
>  	free(rev.message_id);
>  	if (rev.ref_message_ids)
>

I see `char *description_file = NULL` already and it is also used `OPT_FILENAME`. So all good here.

The rest of the patch looks good too. This was commit was quite easy to ready. Appreciate the neat breakdown.

Previous: Patrick SteinhardtNext: Patrick Steinhardt
Message 4 of 74 in “Memory leak fixes (pt.2)”
  1. 00/29 Memory leak fixes (pt.2)Patrick Steinhardt, Jun 3, 2024
  2. 01/29 revision: fix memory leak when reversing revisionsPatrick Steinhardt, Jun 3, 2024
  3. 02/29 parse-options: fix leaks for users of OPT_FILENAMEPatrick Steinhardt, Jun 3, 2024
  4. Karthik NayakJun 6, 2024
  5. 03/29 notes-utils: free note trees when releasing copied notesPatrick Steinhardt, Jun 3, 2024
  6. Karthik NayakJun 6, 2024
  7. 04/29 bundle: plug leaks in `create_bundle()`Patrick Steinhardt, Jun 3, 2024
  8. 05/29 biultin/rev-parse: fix memory leaks in `--parseopt` modePatrick Steinhardt, Jun 3, 2024
  9. 06/29 merge-recursive: fix leaging rename conflict infoPatrick Steinhardt, Jun 3, 2024
  10. Karthik NayakJun 6, 2024
  11. 07/29 revision: fix leaking display notesPatrick Steinhardt, Jun 3, 2024
  12. 08/29 notes: fix memory leak when pruning notesPatrick Steinhardt, Jun 3, 2024
  13. 09/29 builtin/rev-list: fix leaking bitmap index when calculating disk usagePatrick Steinhardt, Jun 3, 2024
  14. 10/29 object-name: free leaking object contextsPatrick Steinhardt, Jun 3, 2024
  15. 11/29 builtin/difftool: plug memory leaks in `run_dir_diff()`Patrick Steinhardt, Jun 3, 2024
  16. 12/29 builtin/merge-recursive: fix leaking object ID basesPatrick Steinhardt, Jun 3, 2024
  17. 13/29 merge-recursive: fix memory leak when finalizing mergePatrick Steinhardt, Jun 3, 2024
  18. Karthik NayakJun 6, 2024
  19. Phillip WoodJun 6, 2024
  20. Karthik NayakJun 12, 2024
  21. 14/29 builtin/log: fix leaking commit list in git-cherry(1)Patrick Steinhardt, Jun 3, 2024
  22. 15/29 revision: free diff optionsPatrick Steinhardt, Jun 3, 2024
  23. 16/29 builtin/stash: fix leak in `show_stash()`Patrick Steinhardt, Jun 3, 2024
  24. 17/29 rerere: fix various trivial leaksPatrick Steinhardt, Jun 3, 2024
  25. 18/29 config: fix leaking "core.notesref" variablePatrick Steinhardt, Jun 3, 2024
  26. 19/29 commit: fix leaking parents when calling `commit_tree_extended()`Patrick Steinhardt, Jun 3, 2024
  27. 20/29 sequencer: fix leaking string buffer in `commit_staged_changes()`Patrick Steinhardt, Jun 3, 2024
  28. Phillip WoodJun 3, 2024
  29. Patrick SteinhardtJun 4, 2024
  30. Patrick SteinhardtJun 4, 2024
  31. Phillip WoodJun 4, 2024
  32. 21/29 apply: fix leaking string in `match_fragment()`Patrick Steinhardt, Jun 3, 2024
  33. 22/29 builtin/clone: plug leaking HEAD ref in `wanted_peer_refs()`Patrick Steinhardt, Jun 3, 2024
  34. 23/29 sequencer: fix memory leaks in `make_script_with_merges()`Patrick Steinhardt, Jun 3, 2024
  35. 24/29 builtin/merge: fix leaking `struct cmdnames` in `get_strategy()`Patrick Steinhardt, Jun 3, 2024
  36. 25/29 merge: fix leaking merge basesPatrick Steinhardt, Jun 3, 2024
  37. 26/29 line-range: plug leaking find functionsPatrick Steinhardt, Jun 3, 2024
  38. 27/29 blame: fix leaking data for blame scoreboardsPatrick Steinhardt, Jun 3, 2024
  39. 28/29 builtin/blame: fix leaking prefixed pathsPatrick Steinhardt, Jun 3, 2024
  40. 29/29 builtin/blame: fix leaking ignore revs filesPatrick Steinhardt, Jun 3, 2024
  41. Karthik NayakJun 6, 2024
  42. Patrick SteinhardtJun 7, 2024
  43. 00/29 Memory leak fixes (pt.2)Patrick Steinhardt, Jun 11, 2024
  44. 01/29 revision: fix memory leak when reversing revisionsPatrick Steinhardt, Jun 11, 2024
  45. 02/29 parse-options: fix leaks for users of OPT_FILENAMEPatrick Steinhardt, Jun 11, 2024
  46. 03/29 notes-utils: free note trees when releasing copied notesPatrick Steinhardt, Jun 11, 2024
  47. 04/29 bundle: plug leaks in `create_bundle()`Patrick Steinhardt, Jun 11, 2024
  48. 05/29 biultin/rev-parse: fix memory leaks in `--parseopt` modePatrick Steinhardt, Jun 11, 2024
  49. 06/29 merge-recursive: fix leaking rename conflict infoPatrick Steinhardt, Jun 11, 2024
  50. 07/29 revision: fix leaking display notesPatrick Steinhardt, Jun 11, 2024
  51. 08/29 notes: fix memory leak when pruning notesPatrick Steinhardt, Jun 11, 2024
  52. 09/29 builtin/rev-list: fix leaking bitmap index when calculating disk usagePatrick Steinhardt, Jun 11, 2024
  53. 10/29 object-name: free leaking object contextsPatrick Steinhardt, Jun 11, 2024
  54. 11/29 builtin/difftool: plug memory leaks in `run_dir_diff()`Patrick Steinhardt, Jun 11, 2024
  55. 12/29 builtin/merge-recursive: fix leaking object ID basesPatrick Steinhardt, Jun 11, 2024
  56. 13/29 merge-recursive: fix memory leak when finalizing mergePatrick Steinhardt, Jun 11, 2024
  57. 14/29 builtin/log: fix leaking commit list in git-cherry(1)Patrick Steinhardt, Jun 11, 2024
  58. 15/29 revision: free diff optionsPatrick Steinhardt, Jun 11, 2024
  59. 16/29 builtin/stash: fix leak in `show_stash()`Patrick Steinhardt, Jun 11, 2024
  60. 17/29 rerere: fix various trivial leaksPatrick Steinhardt, Jun 11, 2024
  61. 18/29 config: fix leaking "core.notesref" variablePatrick Steinhardt, Jun 11, 2024
  62. 19/29 commit: fix leaking parents when calling `commit_tree_extended()`Patrick Steinhardt, Jun 11, 2024
  63. 20/29 sequencer: fix leaking string buffer in `commit_staged_changes()`Patrick Steinhardt, Jun 11, 2024
  64. 21/29 apply: fix leaking string in `match_fragment()`Patrick Steinhardt, Jun 11, 2024
  65. 22/29 builtin/clone: plug leaking HEAD ref in `wanted_peer_refs()`Patrick Steinhardt, Jun 11, 2024
  66. 23/29 sequencer: fix memory leaks in `make_script_with_merges()`Patrick Steinhardt, Jun 11, 2024
  67. 24/29 builtin/merge: fix leaking `struct cmdnames` in `get_strategy()`Patrick Steinhardt, Jun 11, 2024
  68. 25/29 merge: fix leaking merge basesPatrick Steinhardt, Jun 11, 2024
  69. 26/29 line-range: plug leaking find functionsPatrick Steinhardt, Jun 11, 2024
  70. 27/29 blame: fix leaking data for blame scoreboardsPatrick Steinhardt, Jun 11, 2024
  71. 28/29 builtin/blame: fix leaking prefixed pathsPatrick Steinhardt, Jun 11, 2024
  72. 29/29 builtin/blame: fix leaking ignore revs filesPatrick Steinhardt, Jun 11, 2024
  73. Phillip WoodJun 12, 2024
  74. Karthik NayakJun 12, 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.