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

[PATCH 09/12] mailinfo: also free strbuf lists when clearing mailinfo

From
Andrzej Hunt via GitGitGadget <gitgitgadget@gmail.com>
Date
Apr 9, 2021, 18:47 UTC
Message-ID
<130ef89218a47adc7ee558e75672e0e4eb5f30ca.1617994052.git.gitgitgadget@gmail.com>
In-Reply-To
<pull.929.git.1617994052.gitgitgadget@gmail.com>
From: Andrzej Hunt <ajrhunt@google.com>

mailinfo.p_hdr_info/s_hdr_info are null-terminated lists of strbuf's, with entries pointing either to NULL or an allocated strbuf. Therefore we need to free those strbuf's (and not just the data they contain) whenever we're done with a given entry. (See handle_header() where those new strbufs are malloc'd.)

Once we no longer need the list (and not just its entries) we can switch over to strbuf_list_free() instead of manually iterating over the list, which takes care of those additional details for us. We can only do this in clear_mailinfo() - in handle_commit_message() we are only clearing the array contents but want to reuse the array itself, hence we can't use strbuf_list_free() there.

Leak output from t0023:
Direct leak of 72 byte(s) in 3 object(s) allocated from:
    #0 0x49a85d in malloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3
    #1 0x9ac9f4 in do_xmalloc wrapper.c:41:8
    #2 0x9ac9ca in xmalloc wrapper.c:62:9
    #3 0x7f6cf7 in handle_header mailinfo.c:205:10
    #4 0x7f5abf in check_header mailinfo.c:583:4
    #5 0x7f5524 in mailinfo mailinfo.c:1197:3
    #6 0x4dcc95 in parse_mail builtin/am.c:1167:6
    #7 0x4d9070 in am_run builtin/am.c:1732:12
    #8 0x4d5b7a in cmd_am builtin/am.c:2398:3
    #9 0x4cd91d in run_builtin git.c:467:11
    #10 0x4cb5f3 in handle_builtin git.c:719:3
    #11 0x4ccf47 in run_argv git.c:808:4
    #12 0x4caf49 in cmd_main git.c:939:19
    #13 0x69e43e in main common-main.c:52:11
    #14 0x7fc1fadfa349 in __libc_start_main (/lib64/libc.so.6+0x24349)
SUMMARY: AddressSanitizer: 72 byte(s) leaked in 3 allocation(s).
Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
---
 mailinfo.c | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)
diff --git a/mailinfo.c b/mailinfo.c
index 5681d9130db6..95ce191f385b 100644
--- a/mailinfo.c
+++ b/mailinfo.c
@@ -821,7 +821,7 @@ static int handle_commit_msg(struct mailinfo *mi, struct strbuf *line)
 		for (i = 0; header[i]; i++) {
 			if (mi->s_hdr_data[i])
 				strbuf_release(mi->s_hdr_data[i]);
-			mi->s_hdr_data[i] = NULL;
+			FREE_AND_NULL(mi->s_hdr_data[i]);
 		}
 		return 0;
 	}
@@ -1236,22 +1236,14 @@ void setup_mailinfo(struct mailinfo *mi)
 
 void clear_mailinfo(struct mailinfo *mi)
 {
-	int i;
-
 	strbuf_release(&mi->name);
 	strbuf_release(&mi->email);
 	strbuf_release(&mi->charset);
 	strbuf_release(&mi->inbody_header_accum);
 	free(mi->message_id);
 
-	if (mi->p_hdr_data)
-		for (i = 0; mi->p_hdr_data[i]; i++)
-			strbuf_release(mi->p_hdr_data[i]);
-	free(mi->p_hdr_data);
-	if (mi->s_hdr_data)
-		for (i = 0; mi->s_hdr_data[i]; i++)
-			strbuf_release(mi->s_hdr_data[i]);
-	free(mi->s_hdr_data);
+	strbuf_list_free(mi->p_hdr_data);
+	strbuf_list_free(mi->s_hdr_data);
 
 	while (mi->content < mi->content_top) {
 		free(*(mi->content_top));
-- 
gitgitgadget
Previous: Andrzej Hunt via GitGitGadgetNext: Junio C Hamano
Message 16 of 35 in “Fix all leaks in tests t0002-t0099: Part 1”
  1. 00/12 Fix all leaks in tests t0002-t0099: Part 1Andrzej Hunt via GitGitGadget, Apr 9, 2021
  2. 01/12 revision: free remainder of old commit list in limit_listAndrzej Hunt via GitGitGadget, Apr 9, 2021
  3. René ScharfeApr 10, 2021
  4. Andrzej HuntApr 25, 2021
  5. 03/12 ls-files: free max_prefix when doneAndrzej Hunt via GitGitGadget, Apr 9, 2021
  6. René ScharfeApr 10, 2021
  7. Andrzej HuntApr 25, 2021
  8. 02/12 wt-status: fix multiple small leaksAndrzej Hunt via GitGitGadget, Apr 9, 2021
  9. 05/12 branch: FREE_AND_NULL instead of NULL'ing real_refAndrzej Hunt via GitGitGadget, Apr 9, 2021
  10. 04/12 bloom: clear each bloom_key after useAndrzej Hunt via GitGitGadget, Apr 9, 2021
  11. SZEDER GáborApr 11, 2021
  12. Andrzej HuntApr 25, 2021
  13. 06/12 builtin/bugreport: don't leak prefixed filenameAndrzej Hunt via GitGitGadget, Apr 9, 2021
  14. 07/12 builtin/check-ignore: clear_pathspec before returningAndrzej Hunt via GitGitGadget, Apr 9, 2021
  15. 08/12 builtin/checkout: clear pending objects after diffingAndrzej Hunt via GitGitGadget, Apr 9, 2021
  16. 09/12 mailinfo: also free strbuf lists when clearing mailinfoAndrzej Hunt via GitGitGadget, Apr 9, 2021
  17. Junio C HamanoApr 11, 2021
  18. Andrzej HuntApr 25, 2021
  19. 10/12 builtin/for-each-ref: free filter and UNLEAK sorting.Andrzej Hunt via GitGitGadget, Apr 9, 2021
  20. 11/12 builtin/rebase: release git_format_patch_opt tooAndrzej Hunt via GitGitGadget, Apr 9, 2021
  21. 12/12 builtin/rm: avoid leaking pathspec and seenAndrzej Hunt via GitGitGadget, Apr 9, 2021
  22. 00/12 Fix all leaks in tests t0002-t0099: Part 1Andrzej Hunt via GitGitGadget, Apr 25, 2021
  23. 01/12 revision: free remainder of old commit list in limit_listAndrzej Hunt via GitGitGadget, Apr 25, 2021
  24. 02/12 wt-status: fix multiple small leaksAndrzej Hunt via GitGitGadget, Apr 25, 2021
  25. 03/12 ls-files: free max_prefix when doneAndrzej Hunt via GitGitGadget, Apr 25, 2021
  26. 05/12 branch: FREE_AND_NULL instead of NULL'ing real_refAndrzej Hunt via GitGitGadget, Apr 25, 2021
  27. 04/12 bloom: clear each bloom_key after useAndrzej Hunt via GitGitGadget, Apr 25, 2021
  28. 06/12 builtin/bugreport: don't leak prefixed filenameAndrzej Hunt via GitGitGadget, Apr 25, 2021
  29. 07/12 builtin/check-ignore: clear_pathspec before returningAndrzej Hunt via GitGitGadget, Apr 25, 2021
  30. 09/12 mailinfo: also free strbuf lists when clearing mailinfoAndrzej Hunt via GitGitGadget, Apr 25, 2021
  31. Junio C HamanoApr 28, 2021
  32. 10/12 builtin/for-each-ref: free filter and UNLEAK sorting.Andrzej Hunt via GitGitGadget, Apr 25, 2021
  33. 08/12 builtin/checkout: clear pending objects after diffingAndrzej Hunt via GitGitGadget, Apr 25, 2021
  34. 11/12 builtin/rebase: release git_format_patch_opt tooAndrzej Hunt via GitGitGadget, Apr 25, 2021
  35. 12/12 builtin/rm: avoid leaking pathspec and seenAndrzej Hunt via GitGitGadget, Apr 25, 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.