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

[PATCH 04/11] builtin/pack-objects.c: don't leak memory via arguments

From
Taylor Blau <me@ttaylorr.com>
Date
Oct 21, 2021, 03:39 UTC
Message-ID
<dd3b9a949ea3c8c42801e162ed9df045a1848f05.1634787555.git.me@ttaylorr.com>
In-Reply-To
<cover.1634787555.git.me@ttaylorr.com>

When constructing arguments to pass to setup_revision(), pack-objects only frees the memory used by that array after calling get_object_list().

Ensure that we call strvec_clear() whether or not we use the arguments array by cleaning up whenever we exit the function (and rewriting one early return to jump to a label which frees the memory and then returns).

We could avoid setting this array up altogether unless we are in the if-else block that calls get_object_list(), but setting up the argument array is intermingled with lots of other side-effects, e.g.:

    if (exclude_promisor_objects) {
      use_internal_rev_list = 1;
      fetch_if_missing = 0;
      strvec_push(&rp, "--exclude-promisor-objects");
    }

So it would be awkward to check exclude_promisor_objects twice: first to set use_internal_rev_list and fetch_if_missing, and then again above get_object_list() to push the relevant argument onto the array.

Instead, leave the array's construction alone and make sure to free it unconditionally.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
 builtin/pack-objects.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 1a3dd445f8..857be7826f 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -4148,11 +4148,10 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
 		read_packs_list_from_stdin();
 		if (rev_list_unpacked)
 			add_unreachable_loose_objects();
-	} else if (!use_internal_rev_list)
+	} else if (!use_internal_rev_list) {
 		read_object_list_from_stdin();
-	else {
+	} else {
 		get_object_list(rp.nr, rp.v);
-		strvec_clear(&rp);
 	}
 	cleanup_preferred_base();
 	if (include_tag && nr_result)
@@ -4162,7 +4161,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
 			    the_repository);
 
 	if (non_empty && !nr_result)
-		return 0;
+		goto cleanup;
 	if (nr_result) {
 		trace2_region_enter("pack-objects", "prepare-pack",
 				    the_repository);
@@ -4183,5 +4182,9 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
 			     " pack-reused %"PRIu32),
 			   written, written_delta, reused, reused_delta,
 			   reuse_packfile_objects);
+
+cleanup:
+	strvec_clear(&rp);
+
 	return 0;
 }
-- 
2.33.0.96.g73915697e6
Previous: Taylor BlauNext: Taylor Blau
Message 12 of 63 in “midx: clean up t5319 under 'SANITIZE=leak'”
  1. 00/11 midx: clean up t5319 under 'SANITIZE=leak'Taylor Blau, Oct 21, 2021
  2. 01/11 midx.c: clean up chunkfile after reading the MIDXTaylor Blau, Oct 21, 2021
  3. Junio C HamanoOct 21, 2021
  4. Ævar Arnfjörð BjarmasonOct 21, 2021
  5. Junio C HamanoOct 21, 2021
  6. Taylor BlauOct 22, 2021
  7. 02/11 midx.c: don't leak MIDX from verify_midx_fileTaylor Blau, Oct 21, 2021
  8. Eric SunshineOct 21, 2021
  9. Junio C HamanoOct 21, 2021
  10. Junio C HamanoOct 21, 2021
  11. 03/11 t/helper/test-read-midx.c: free MIDX within read_midx_file()Taylor Blau, Oct 21, 2021
  12. 04/11 builtin/pack-objects.c: don't leak memory via argumentsTaylor Blau, Oct 21, 2021
  13. 05/11 builtin/repack.c: avoid leaking child argumentsTaylor Blau, Oct 21, 2021
  14. Derrick StoleeOct 21, 2021
  15. Junio C HamanoOct 21, 2021
  16. Junio C HamanoOct 21, 2021
  17. Taylor BlauOct 22, 2021
  18. 06/11 builtin/multi-pack-index.c: don't leak concatenated optionsTaylor Blau, Oct 21, 2021
  19. 07/11 pack-bitmap.c: avoid leaking via midx_bitmap_filename()Taylor Blau, Oct 21, 2021
  20. Junio C HamanoOct 21, 2021
  21. Taylor BlauOct 22, 2021
  22. 08/11 pack-bitmap.c: don't leak type-level bitmapsTaylor Blau, Oct 21, 2021
  23. Junio C HamanoOct 21, 2021
  24. 09/11 pack-bitmap.c: more aggressively free in free_bitmap_index()Taylor Blau, Oct 21, 2021
  25. Eric SunshineOct 21, 2021
  26. Junio C HamanoOct 21, 2021
  27. Taylor BlauOct 22, 2021
  28. Junio C HamanoOct 21, 2021
  29. 11/11 t5319: UNLEAK() the remaining leaksTaylor Blau, Oct 21, 2021
  30. 10/11 pack-bitmap-write.c: don't return without stop_progress()Taylor Blau, Oct 21, 2021
  31. Eric SunshineOct 21, 2021
  32. Ævar Arnfjörð BjarmasonOct 21, 2021
  33. Junio C HamanoOct 21, 2021
  34. Taylor BlauOct 22, 2021
  35. Junio C HamanoOct 23, 2021
  36. SubmittingPatchs: clarify choice of base and testingJunio C Hamano, Oct 23, 2021
  37. Ævar Arnfjörð BjarmasonOct 23, 2021
  38. Junio C HamanoOct 23, 2021
  39. Junio C HamanoOct 23, 2021
  40. Fabian StelzerOct 25, 2021
  41. Junio C HamanoOct 25, 2021
  42. Junio C HamanoOct 25, 2021
  43. Junio C HamanoOct 25, 2021
  44. SubmittingPatchs: clarify choice of base and testingJunio C Hamano, Dec 23, 2021
  45. Elijah NewrenDec 28, 2021
  46. Fabian StelzerDec 30, 2021
  47. Re* [PATCH v2] SubmittingPatchs: clarify choice of base and testingJunio C Hamano, Dec 30, 2021
  48. Ævar Arnfjörð BjarmasonOct 21, 2021
  49. Taylor BlauOct 22, 2021
  50. Ævar Arnfjörð BjarmasonOct 22, 2021
  51. leak tests: add an interface to the LSAN_OPTIONS "suppressions"Ævar Arnfjörð Bjarmason, Oct 22, 2021
  52. Taylor BlauOct 26, 2021
  53. Jeff KingOct 26, 2021
  54. Taylor BlauOct 26, 2021
  55. Jeff KingOct 26, 2021
  56. Ævar Arnfjörð BjarmasonOct 27, 2021
  57. Jeff KingOct 27, 2021
  58. Junio C HamanoOct 27, 2021
  59. Ævar Arnfjörð BjarmasonOct 27, 2021
  60. Jeff KingOct 29, 2021
  61. Jeff KingOct 29, 2021
  62. Ævar Arnfjörð BjarmasonOct 27, 2021
  63. Derrick StoleeOct 21, 2021

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.