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

[PATCH 28/28] http-push: clean up local_refs at exit

From
Jeff King <peff@peff.net>
Date
Sep 24, 2024, 22:12 UTC
Message-ID
<20240924221239.GB1143820@coredump.intra.peff.net>
In-Reply-To
<20240924214930.GA1143523@coredump.intra.peff.net>

We allocate a list of ref structs from get_local_heads() but never clean it up. We should do so before exiting to avoid complaints from the leak-checker. Note that we have to initialize it to NULL, because there's one code path that can jump to the cleanup label before we assign to it.

Fixing this lets us mark t5540 as leak-free.

Curiously building with SANITIZE=leak and gcc does not seem to find this problem, but switching to clang does. It seems like a fairly obvious leak, though.

I was curious that the matching remote_refs did not have the same leak. But that is because we store the list in a global variable, so it's still reachable after we exit. Arguably we could treat it the same as future-proofing, but I didn't bother (now that the script is marked leak-free, anybody moving it to a stack variable will notice).

Signed-off-by: Jeff King <peff@peff.net>
---
 http-push.c                 | 3 ++-
 t/t5540-http-push-webdav.sh | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/http-push.c b/http-push.c
index b36b1f9e35..aad89f2eab 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1719,7 +1719,7 @@ int cmd_main(int argc, const char **argv)
 	int rc = 0;
 	int i;
 	int new_refs;
-	struct ref *ref, *local_refs;
+	struct ref *ref, *local_refs = NULL;
 
 	CALLOC_ARRAY(repo, 1);
 
@@ -1997,6 +1997,7 @@ int cmd_main(int argc, const char **argv)
 	}
 
 	refspec_clear(&rs);
+	free_refs(local_refs);
 
 	return rc;
 }
diff --git a/t/t5540-http-push-webdav.sh b/t/t5540-http-push-webdav.sh
index 37db3dec0c..27389b0908 100755
--- a/t/t5540-http-push-webdav.sh
+++ b/t/t5540-http-push-webdav.sh
@@ -10,6 +10,7 @@ This test runs various sanity checks on http-push.'
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 if git http-push > /dev/null 2>&1 || [ $? -eq 128 ]
-- 
2.46.2.1011.gf1f9323e02
Previous: Jeff KingNext: Patrick Steinhardt
Message 47 of 49 in “leak fixes for http fetch/push”
  1. 0/28 leak fixes for http fetch/pushJeff King, Sep 24, 2024
  2. 01/28 http-fetch: clear leaking git-index-pack(1) argumentsJeff King, Sep 24, 2024
  3. 02/28 shallow: fix leak when unregistering last shallow rootJeff King, Sep 24, 2024
  4. 03/28 fetch-pack: fix leaking sought refsJeff King, Sep 24, 2024
  5. René ScharfeSep 25, 2024
  6. Patrick SteinhardtSep 26, 2024
  7. 04/28 connect: clear child process before freeing in diagnostic modeJeff King, Sep 24, 2024
  8. Patrick SteinhardtSep 26, 2024
  9. 05/28 fetch-pack: free object filter before exitingJeff King, Sep 24, 2024
  10. Patrick SteinhardtSep 26, 2024
  11. 06/28 fetch-pack, send-pack: clean up shallow oid arrayJeff King, Sep 24, 2024
  12. Patrick SteinhardtSep 26, 2024
  13. Jeff KingSep 27, 2024
  14. 07/28 commit: avoid leaking already-saved bufferJeff King, Sep 24, 2024
  15. Patrick SteinhardtSep 26, 2024
  16. 08/28 send-pack: free cas options before exitJeff King, Sep 24, 2024
  17. Patrick SteinhardtSep 26, 2024
  18. Jeff KingSep 27, 2024
  19. 09/28 transport-helper: fix strbuf leak in push_refs_with_push()Jeff King, Sep 24, 2024
  20. Patrick SteinhardtSep 26, 2024
  21. Jeff KingSep 27, 2024
  22. 10/28 fetch: free "raw" string when shrinking refspecJeff King, Sep 24, 2024
  23. 11/28 fetch-pack: clear pack lockfiles listJeff King, Sep 24, 2024
  24. 12/28 transport-helper: fix leak of dummy refs_listJeff King, Sep 24, 2024
  25. 13/28 http: fix leak when redacting cookies from curl traceJeff King, Sep 24, 2024
  26. 14/28 http: fix leak of http_object_request structJeff King, Sep 24, 2024
  27. Patrick SteinhardtSep 26, 2024
  28. Jeff KingSep 27, 2024
  29. 15/28 http: call git_inflate_end() when releasing http_object_requestJeff King, Sep 24, 2024
  30. Patrick SteinhardtSep 26, 2024
  31. Jeff KingSep 27, 2024
  32. 16/28 http: stop leaking buffer in http_get_info_packs()Jeff King, Sep 24, 2024
  33. 17/28 remote-curl: free HEAD ref with free_one_ref()Jeff King, Sep 24, 2024
  34. 18/28 http-walker: free fake packed_git listJeff King, Sep 24, 2024
  35. 19/28 http-push: clear refspecs before exitingJeff King, Sep 24, 2024
  36. 20/28 http-push: free repo->url stringJeff King, Sep 24, 2024
  37. Patrick SteinhardtSep 26, 2024
  38. Jeff KingSep 27, 2024
  39. 21/28 http-push: free curl header listsJeff King, Sep 24, 2024
  40. Patrick SteinhardtSep 26, 2024
  41. 22/28 http-push: free transfer_request dest fieldJeff King, Sep 24, 2024
  42. 23/28 http-push: free transfer_request strbufJeff King, Sep 24, 2024
  43. 24/28 http-push: free remote_ls_ctx.dentry_nameJeff King, Sep 24, 2024
  44. 25/28 http-push: free xml_ctx.cdata after useJeff King, Sep 24, 2024
  45. 26/28 http-push: clean up objects listJeff King, Sep 24, 2024
  46. 27/28 http-push: clean up loose request when falling back to packedJeff King, Sep 24, 2024
  47. 28/28 http-push: clean up local_refs at exitJeff King, Sep 24, 2024
  48. Patrick SteinhardtSep 26, 2024
  49. Jeff KingSep 27, 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.