diff mbox series

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

Message ID 20240924221239.GB1143820@coredump.intra.peff.net (mailing list archive)
State Accepted
Commit f4c768c639566da07fc063fa9b52607f54ac94ee
Headers show
Series leak fixes for http fetch/push | expand

Commit Message

Jeff King Sept. 24, 2024, 10:12 p.m. UTC
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 mbox series

Patch

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 ]