Merge pull request #2753 from ethomson/win32_leaks

Win32 leaks

5 files changed, 122 insertions, 55 deletions

diff --git a/src/indexer.c b/src/indexer.c
index 8127ff8b0..92f85c167 100644
--- a/src/indexer.c
+++ b/src/indexer.c
@@ -1028,6 +1028,7 @@ int git_indexer_commit(git_indexer *idx, git_transfer_progress *stats)
 	p_rename(idx->pack->pack_name, git_buf_cstr(&filename));
 
 	git_buf_free(&filename);
+	git_hash_ctx_cleanup(&ctx);
 	return 0;
 
 on_error:
diff --git a/src/odb.c b/src/odb.c
index 5961b35a7..d2dff7178 100644
--- a/src/odb.c
+++ b/src/odb.c
@@ -1045,6 +1045,7 @@ void git_odb_stream_free(git_odb_stream *stream)
 	if (stream == NULL)
 		return;
 
+	git_hash_ctx_cleanup(stream->hash_ctx);
 	git__free(stream->hash_ctx);
 	stream->free(stream);
 }
diff --git a/src/transports/winhttp.c b/src/transports/winhttp.c
index 032ac51cb..56b7c99f9 100644
--- a/src/transports/winhttp.c
+++ b/src/transports/winhttp.c
@@ -241,6 +241,31 @@ static int certificate_check(winhttp_stream *s, int valid)
 	return error;
 }
 
+static void winhttp_stream_close(winhttp_stream *s)
+{
+	if (s->chunk_buffer) {
+		git__free(s->chunk_buffer);
+		s->chunk_buffer = NULL;
+	}
+
+	if (s->post_body) {
+		CloseHandle(s->post_body);
+		s->post_body = NULL;
+	}
+
+	if (s->request_uri) {
+		git__free(s->request_uri);
+		s->request_uri = NULL;
+	}
+
+	if (s->request) {
+		WinHttpCloseHandle(s->request);
+		s->request = NULL;
+	}
+
+	s->sent_request = 0;
+}
+
 static int winhttp_stream_connect(winhttp_stream *s)
 {
 	winhttp_subtransport *t = OWNING_SUBTRANSPORT(s);
@@ -420,6 +445,9 @@ static int winhttp_stream_connect(winhttp_stream *s)
 	error = 0;
 
 on_error:
+	if (error < 0)
+		winhttp_stream_close(s);
+
 	git__free(proxy_url);
 	git_buf_free(&buf);
 	return error;
@@ -497,6 +525,31 @@ static int write_chunk(HINTERNET request, const char *buffer, size_t len)
 	return 0;
 }
 
+static int winhttp_close_connection(winhttp_subtransport *t)
+{
+	int ret = 0;
+
+	if (t->connection) {
+		if (!WinHttpCloseHandle(t->connection)) {
+			giterr_set(GITERR_OS, "Unable to close connection");
+			ret = -1;
+		}
+
+		t->connection = NULL;
+	}
+
+	if (t->session) {
+		if (!WinHttpCloseHandle(t->session)) {
+			giterr_set(GITERR_OS, "Unable to close session");
+			ret = -1;
+		}
+
+		t->session = NULL;
+	}
+
+	return ret;
+}
+
 static int winhttp_connect(
 	winhttp_subtransport *t,
 	const char *url)
@@ -510,6 +563,9 @@ static int winhttp_connect(
 
 	GIT_UNUSED(url);
 
+	t->session = NULL;
+	t->connection = NULL;
+
 	/* Prepare port */
 	if (git__strtol32(&port, t->connection_data.port, NULL, 10) < 0)
 		return -1;
@@ -554,6 +610,9 @@ static int winhttp_connect(
 	error = 0;
 
 on_error:
+	if (error < 0)
+		winhttp_close_connection(t);
+
 	git__free(wide_host);
 
 	return error;
@@ -790,9 +849,7 @@ replay:
 			git__free(location);
 
 			/* Replay the request */
-			WinHttpCloseHandle(s->request);
-			s->request = NULL;
-			s->sent_request = 0;
+			winhttp_stream_close(s);
 
 			if (!git__prefixcmp_icase(location8, prefix_https)) {
 				/* Upgrade to secure connection; disconnect and start over */
@@ -801,7 +858,10 @@ replay:
 					return -1;
 				}
 
-				winhttp_connect(t, location8);
+				winhttp_close_connection(t);
+
+				if (winhttp_connect(t, location8) < 0)
+					return -1;
 			}
 
 			git__free(location8);
@@ -840,9 +900,7 @@ replay:
 				if (!cred_error) {
 					assert(t->cred);
 
-					WinHttpCloseHandle(s->request);
-					s->request = NULL;
-					s->sent_request = 0;
+					winhttp_stream_close(s);
 
 					/* Successfully acquired a credential */
 					goto replay;
@@ -1109,26 +1167,7 @@ static void winhttp_stream_free(git_smart_subtransport_stream *stream)
 {
 	winhttp_stream *s = (winhttp_stream *)stream;
 
-	if (s->chunk_buffer) {
-		git__free(s->chunk_buffer);
-		s->chunk_buffer = NULL;
-	}
-
-	if (s->post_body) {
-		CloseHandle(s->post_body);
-		s->post_body = NULL;
-	}
-
-	if (s->request_uri) {
-		git__free(s->request_uri);
-		s->request_uri = NULL;
-	}
-
-	if (s->request) {
-		WinHttpCloseHandle(s->request);
-		s->request = NULL;
-	}
-
+	winhttp_stream_close(s);
 	git__free(s);
 }
 
@@ -1257,7 +1296,6 @@ static int winhttp_action(
 static int winhttp_close(git_smart_subtransport *subtransport)
 {
 	winhttp_subtransport *t = (winhttp_subtransport *)subtransport;
-	int ret = 0;
 
 	gitno_connection_data_free_ptrs(&t->connection_data);
 	memset(&t->connection_data, 0x0, sizeof(gitno_connection_data));
@@ -1272,25 +1310,7 @@ static int winhttp_close(git_smart_subtransport *subtransport)
 		t->url_cred = NULL;
 	}
 
-	if (t->connection) {
-		if (!WinHttpCloseHandle(t->connection)) {
-			giterr_set(GITERR_OS, "Unable to close connection");
-			ret = -1;
-		}
-
-		t->connection = NULL;
-	}
-
-	if (t->session) {
-		if (!WinHttpCloseHandle(t->session)) {
-			giterr_set(GITERR_OS, "Unable to close session");
-			ret = -1;
-		}
-
-		t->session = NULL;
-	}
-
-	return ret;
+	return winhttp_close_connection(t);
 }
 
 static void winhttp_free(git_smart_subtransport *subtransport)
diff --git a/tests/clar_libgit2.c b/tests/clar_libgit2.c
index 9640b9265..10f37ad5c 100644
--- a/tests/clar_libgit2.c
+++ b/tests/clar_libgit2.c
@@ -80,7 +80,7 @@ char *cl_getenv(const char *name)
 
 int cl_setenv(const char *name, const char *value)
 {
-	wchar_t *wide_name, *wide_value;
+	wchar_t *wide_name, *wide_value = NULL;
 
 	cl_assert(git__utf8_to_16_alloc(&wide_name, name) >= 0);
 
@@ -95,6 +95,8 @@ int cl_setenv(const char *name, const char *value)
 		SetEnvironmentVariableW(wide_name, NULL);
 	}
 
+	git__free(wide_name);
+	git__free(wide_value);
 	return 0;
 }
 
diff --git a/tests/core/link.c b/tests/core/link.c
index 1794a3893..83999ebdf 100644
--- a/tests/core/link.c
+++ b/tests/core/link.c
@@ -23,7 +23,7 @@ void test_core_link__cleanup(void)
 }
 
 #ifdef GIT_WIN32
-static bool is_administrator(void)
+static bool should_run(void)
 {
 	static SID_IDENTIFIER_AUTHORITY authority = { SECURITY_NT_AUTHORITY };
 	PSID admin_sid;
@@ -35,6 +35,11 @@ static bool is_administrator(void)
 
 	return is_admin ? true : false;
 }
+#else
+static bool should_run(void)
+{
+	return true;
+}
 #endif
 
 static void do_symlink(const char *old, const char *new, int is_dir)
@@ -48,9 +53,6 @@ static void do_symlink(const char *old, const char *new, int is_dir)
 	HMODULE module;
 	create_symlink_func pCreateSymbolicLink;
 
-	if (!is_administrator())
-		clar__skip();
-
 	cl_assert(module = GetModuleHandle("kernel32"));
 	cl_assert(pCreateSymbolicLink = (create_symlink_func)GetProcAddress(module, "CreateSymbolicLinkA"));
 
@@ -67,9 +69,6 @@ static void do_hardlink(const char *old, const char *new)
 	HMODULE module;
 	create_hardlink_func pCreateHardLink;
 
-	if (!is_administrator())
-		clar__skip();
-
 	cl_assert(module = GetModuleHandle("kernel32"));
 	cl_assert(pCreateHardLink = (create_hardlink_func)GetProcAddress(module, "CreateHardLinkA"));
 
@@ -147,6 +146,8 @@ static void do_junction(const char *old, const char *new)
 
 	CloseHandle(handle);
 	LocalFree(reparse_buf);
+
+	git_buf_free(&unparsed_buf);
 }
 
 static void do_custom_reparse(const char *path)
@@ -235,6 +236,9 @@ void test_core_link__stat_symlink(void)
 {
 	struct stat st;
 
+	if (!should_run())
+		clar__skip();
+
 	cl_git_rewritefile("stat_target", "This is the target of a symbolic link.\n");
 	do_symlink("stat_target", "stat_symlink", 0);
 
@@ -251,6 +255,9 @@ void test_core_link__stat_symlink_directory(void)
 {
 	struct stat st;
 
+	if (!should_run())
+		clar__skip();
+
 	p_mkdir("stat_dirtarget", 0777);
 	do_symlink("stat_dirtarget", "stat_dirlink", 1);
 
@@ -265,6 +272,9 @@ void test_core_link__stat_symlink_chain(void)
 {
 	struct stat st;
 
+	if (!should_run())
+		clar__skip();
+
 	cl_git_rewritefile("stat_final_target", "Final target of some symbolic links...\n");
 	do_symlink("stat_final_target", "stat_chain_3", 0);
 	do_symlink("stat_chain_3", "stat_chain_2", 0);
@@ -279,6 +289,9 @@ void test_core_link__stat_dangling_symlink(void)
 {
 	struct stat st;
 
+	if (!should_run())
+		clar__skip();
+
 	do_symlink("stat_nonexistent", "stat_dangling", 0);
 
 	cl_must_fail(p_stat("stat_nonexistent", &st));
@@ -289,6 +302,9 @@ void test_core_link__stat_dangling_symlink_directory(void)
 {
 	struct stat st;
 
+	if (!should_run())
+		clar__skip();
+
 	do_symlink("stat_nonexistent", "stat_dangling_dir", 1);
 
 	cl_must_fail(p_stat("stat_nonexistent_dir", &st));
@@ -300,6 +316,9 @@ void test_core_link__lstat_symlink(void)
 	git_buf target_path = GIT_BUF_INIT;
 	struct stat st;
 
+	if (!should_run())
+		clar__skip();
+
 	/* Windows always writes the canonical path as the link target, so
 	 * write the full path on all platforms.
 	 */
@@ -324,6 +343,9 @@ void test_core_link__lstat_symlink_directory(void)
 	git_buf target_path = GIT_BUF_INIT;
 	struct stat st;
 
+	if (!should_run())
+		clar__skip();
+
 	git_buf_join(&target_path, '/', clar_sandbox_path(), "lstat_dirtarget");
 
 	p_mkdir("lstat_dirtarget", 0777);
@@ -343,6 +365,9 @@ void test_core_link__lstat_dangling_symlink(void)
 {
 	struct stat st;
 
+	if (!should_run())
+		clar__skip();
+
 	do_symlink("lstat_nonexistent", "lstat_dangling", 0);
 
 	cl_must_fail(p_lstat("lstat_nonexistent", &st));
@@ -356,6 +381,9 @@ void test_core_link__lstat_dangling_symlink_directory(void)
 {
 	struct stat st;
 
+	if (!should_run())
+		clar__skip();
+
 	do_symlink("lstat_nonexistent", "lstat_dangling_dir", 1);
 
 	cl_must_fail(p_lstat("lstat_nonexistent", &st));
@@ -454,6 +482,9 @@ void test_core_link__stat_hardlink(void)
 {
 	struct stat st;
 
+	if (!should_run())
+		clar__skip();
+
 	cl_git_rewritefile("stat_hardlink1", "This file has many names!\n");
 	do_hardlink("stat_hardlink1", "stat_hardlink2");
 
@@ -470,6 +501,9 @@ void test_core_link__lstat_hardlink(void)
 {
 	struct stat st;
 
+	if (!should_run())
+		clar__skip();
+
 	cl_git_rewritefile("lstat_hardlink1", "This file has many names!\n");
 	do_hardlink("lstat_hardlink1", "lstat_hardlink2");
 
@@ -537,6 +571,9 @@ void test_core_link__readlink_symlink(void)
 	int len;
 	char buf[2048];
 
+	if (!should_run())
+		clar__skip();
+
 	git_buf_join(&target_path, '/', clar_sandbox_path(), "readlink_target");
 
 	cl_git_rewritefile("readlink_target", "This is the target of a symlink\n");
@@ -558,6 +595,9 @@ void test_core_link__readlink_dangling(void)
 	int len;
 	char buf[2048];
 
+	if (!should_run())
+		clar__skip();
+
 	git_buf_join(&target_path, '/', clar_sandbox_path(), "readlink_nonexistent");
 
 	do_symlink(git_buf_cstr(&target_path), "readlink_dangling", 0);
@@ -579,6 +619,9 @@ void test_core_link__readlink_multiple(void)
 	int len;
 	char buf[2048];
 
+	if (!should_run())
+		clar__skip();
+
 	git_buf_join(&target_path, '/', clar_sandbox_path(), "readlink_final");
 	git_buf_join(&path3, '/', clar_sandbox_path(), "readlink_3");
 	git_buf_join(&path2, '/', clar_sandbox_path(), "readlink_2");