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

[PATCH v2 13/24] builtin/rev-parse: fix memory leak with `--parseopt`

From
Patrick Steinhardt <ps@pks.im>
Date
Aug 1, 2024, 10:40 UTC
Message-ID
<034c416d4693701cecc3201b9f8d9839fd99c1ab.1722499961.git.ps@pks.im>
In-Reply-To
<cover.1722499961.git.ps@pks.im>

The `--parseopt` mode allows shell scripts to have the same option parsing mode as we have in C builtins. It soaks up a set of option descriptions via stdin and massages them into proper `struct option`s that we can then use to parse a set of arguments.

We only partially free those options when done though, creating a memory leak. Interestingly, we only end up free'ing the first option's help, which is of course wrong.

Fix this by freeing all option's help fields as well as their `argh` fields to plug this memory leak.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/rev-parse.c           | 5 ++++-
 t/t1502-rev-parse-parseopt.sh | 2 ++
 2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 2e64f5bda7..5845d3f59b 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -553,7 +553,10 @@ static int cmd_parseopt(int argc, const char **argv, const char *prefix)
 	strbuf_release(&sb);
 	strvec_clear(&longnames);
 	strvec_clear(&usage);
-	free((char *) opts->help);
+	for (size_t i = 0; i < opts_nr; i++) {
+		free((char *) opts[i].help);
+		free((char *) opts[i].argh);
+	}
 	free(opts);
 	return 0;
 }
diff --git a/t/t1502-rev-parse-parseopt.sh b/t/t1502-rev-parse-parseopt.sh
index b754b9fd74..5eaa6428c4 100755
--- a/t/t1502-rev-parse-parseopt.sh
+++ b/t/t1502-rev-parse-parseopt.sh
@@ -1,6 +1,8 @@
 #!/bin/sh
 
 test_description='test git rev-parse --parseopt'
+
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 check_invalid_long_option () {
-- 
2.46.0.dirty
Previous: Patrick SteinhardtNext: Patrick Steinhardt
Message 59 of 72 in “Memory leak fixes (pt.3)”
  1. 00/23 Memory leak fixes (pt.3)Patrick Steinhardt, Jul 26, 2024
  2. 01/23 builtin/replay: plug leaking `advance_name` variablePatrick Steinhardt, Jul 26, 2024
  3. Taylor BlauJul 31, 2024
  4. 02/23 builtin/log: fix leaking branch name when creating cover lettersPatrick Steinhardt, Jul 26, 2024
  5. Karthik NayakJul 30, 2024
  6. Taylor BlauJul 31, 2024
  7. 03/23 builtin/describe: fix memory leak with `--contains=`Patrick Steinhardt, Jul 26, 2024
  8. Karthik NayakJul 30, 2024
  9. Junio C HamanoJul 30, 2024
  10. Patrick SteinhardtJul 31, 2024
  11. Junio C HamanoJul 31, 2024
  12. Taylor BlauJul 31, 2024
  13. 04/23 builtin/describe: fix leaking array when running diff-indexPatrick Steinhardt, Jul 26, 2024
  14. Karthik NayakJul 30, 2024
  15. 05/23 builtin/describe: fix trivial memory leak when describing blobPatrick Steinhardt, Jul 26, 2024
  16. 06/23 builtin/name-rev: fix various trivial memory leaksPatrick Steinhardt, Jul 26, 2024
  17. Junio C HamanoJul 30, 2024
  18. 07/23 builtin/submodule--helper: fix various trivial memory leaksPatrick Steinhardt, Jul 26, 2024
  19. Rubén JustoJul 31, 2024
  20. Patrick SteinhardtAug 1, 2024
  21. 08/23 builtin/ls-remote: fix leaking `pattern` stringsPatrick Steinhardt, Jul 26, 2024
  22. Taylor BlauJul 31, 2024
  23. Patrick SteinhardtAug 1, 2024
  24. 09/23 builtin/remote: fix leaking strings in `branch_list`Patrick Steinhardt, Jul 26, 2024
  25. Taylor BlauJul 31, 2024
  26. 10/23 builtin/remote: fix various trivial memory leaksPatrick Steinhardt, Jul 26, 2024
  27. 11/23 builtin/stash: fix various trivial memory leaksPatrick Steinhardt, Jul 26, 2024
  28. Taylor BlauJul 31, 2024
  29. 12/23 builtin/rev-parse: fix memory leak with `--parseopt`Patrick Steinhardt, Jul 26, 2024
  30. Karthik NayakJul 30, 2024
  31. 13/23 builtin/show-branch: fix several memory leaksPatrick Steinhardt, Jul 26, 2024
  32. 14/23 builtin/credential-store: fix leaking credentialPatrick Steinhardt, Jul 26, 2024
  33. 15/23 builtin/rerere: fix various trivial memory leaksPatrick Steinhardt, Jul 26, 2024
  34. 16/23 builtin/shortlog: fix various trivial memory leaksPatrick Steinhardt, Jul 26, 2024
  35. 17/23 builtin/worktree: fix leaking derived branch namesPatrick Steinhardt, Jul 26, 2024
  36. 18/23 builtin/credential-cache: fix trivial leaksPatrick Steinhardt, Jul 26, 2024
  37. 19/23 t/test-repository: fix leaking repositoryPatrick Steinhardt, Jul 26, 2024
  38. 20/23 object-name: fix leaking commit list itemsPatrick Steinhardt, Jul 26, 2024
  39. 21/23 entry: fix leaking pathnames during delayed checkoutPatrick Steinhardt, Jul 26, 2024
  40. 22/23 convert: fix leaking config stringsPatrick Steinhardt, Jul 26, 2024
  41. 23/23 commit-reach: fix trivial memory leak when computing reachabilityPatrick Steinhardt, Jul 26, 2024
  42. Karthik NayakJul 30, 2024
  43. Patrick SteinhardtJul 31, 2024
  44. Taylor BlauJul 31, 2024
  45. Patrick SteinhardtAug 1, 2024
  46. Taylor BlauAug 1, 2024
  47. 00/24 Memory leak fixes (pt.3)Patrick Steinhardt, Aug 1, 2024
  48. 01/24 builtin/replay: plug leaking `advance_name` variablePatrick Steinhardt, Aug 1, 2024
  49. 02/24 builtin/log: fix leaking branch name when creating cover lettersPatrick Steinhardt, Aug 1, 2024
  50. 03/24 builtin/describe: fix memory leak with `--contains=`Patrick Steinhardt, Aug 1, 2024
  51. 04/24 builtin/describe: fix leaking array when running diff-indexPatrick Steinhardt, Aug 1, 2024
  52. 05/24 builtin/describe: fix trivial memory leak when describing blobPatrick Steinhardt, Aug 1, 2024
  53. 06/24 builtin/name-rev: fix various trivial memory leaksPatrick Steinhardt, Aug 1, 2024
  54. 08/24 builtin/submodule--helper: fix leaking buffer in `is_tip_reachable`Patrick Steinhardt, Aug 1, 2024
  55. 09/24 builtin/ls-remote: fix leaking `pattern` stringsPatrick Steinhardt, Aug 1, 2024
  56. 10/24 builtin/remote: fix leaking strings in `branch_list`Patrick Steinhardt, Aug 1, 2024
  57. 11/24 builtin/remote: fix various trivial memory leaksPatrick Steinhardt, Aug 1, 2024
  58. 12/24 builtin/stash: fix various trivial memory leaksPatrick Steinhardt, Aug 1, 2024
  59. 13/24 builtin/rev-parse: fix memory leak with `--parseopt`Patrick Steinhardt, Aug 1, 2024
  60. 14/24 builtin/show-branch: fix several memory leaksPatrick Steinhardt, Aug 1, 2024
  61. 15/24 builtin/credential-store: fix leaking credentialPatrick Steinhardt, Aug 1, 2024
  62. 16/24 builtin/rerere: fix various trivial memory leaksPatrick Steinhardt, Aug 1, 2024
  63. 17/24 builtin/shortlog: fix various trivial memory leaksPatrick Steinhardt, Aug 1, 2024
  64. 18/24 builtin/worktree: fix leaking derived branch namesPatrick Steinhardt, Aug 1, 2024
  65. 19/24 builtin/credential-cache: fix trivial leaksPatrick Steinhardt, Aug 1, 2024
  66. 20/24 t/test-repository: fix leaking repositoryPatrick Steinhardt, Aug 1, 2024
  67. 21/24 object-name: fix leaking commit list itemsPatrick Steinhardt, Aug 1, 2024
  68. 22/24 entry: fix leaking pathnames during delayed checkoutPatrick Steinhardt, Aug 1, 2024
  69. 23/24 convert: fix leaking config stringsPatrick Steinhardt, Aug 1, 2024
  70. 24/24 commit-reach: fix trivial memory leak when computing reachabilityPatrick Steinhardt, Aug 1, 2024
  71. 07/24 builtin/submodule--helper: fix leaking clone depth parameterPatrick Steinhardt, Aug 1, 2024
  72. Taylor BlauAug 1, 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.