Fix gitno_connect() error handling on Windows

gitno_connect() can return an error or socket, which is fine on most
platforms where sockets are file descriptors (signed int), but on Windows,
SOCKET is an unsigned type, which is problematic when we are trying to
test if the socket was actually a negative error code.

This fix seperates the error code and socket in gitno_connect(), and fixes
the error handling in do_connect() functions to compensate. It appears
that git_connect() and the git-transport do_connect() functions had bugs
in the non-windows cases too (leaking sockets, and not properly reporting
connection error, respectively) so I went ahead and fixed those too.

2 files changed, 11 insertions, 8 deletions

diff --git a/src/transports/git.c b/src/transports/git.c
index 88e7e8160..b0acc7e62 100644
--- a/src/transports/git.c
+++ b/src/transports/git.c
@@ -106,10 +106,12 @@ static int do_connect(transport_git *t, const char *url)
 	if (error < GIT_SUCCESS)
 		return error;
 
-	s = gitno_connect(host, port);
-	connected = 1;
-	error = send_request(s, NULL, url);
-	t->socket = s;
+	error = gitno_connect(host, port, &s);
+	if (error == GIT_SUCCESS) {
+		connected = 1;
+		error = send_request(s, NULL, url);
+		t->socket = s;
+	}
 
 	git__free(host);
 	git__free(port);
diff --git a/src/transports/http.c b/src/transports/http.c
index 2842d08fd..323b56105 100644
--- a/src/transports/http.c
+++ b/src/transports/http.c
@@ -82,14 +82,15 @@ static int gen_request(git_buf *buf, const char *url, const char *host, const ch
 
 static int do_connect(transport_http *t, const char *host, const char *port)
 {
-	GIT_SOCKET s = -1;
+	int error = GIT_SUCCESS;
+	GIT_SOCKET s;
 
 	if (t->parent.connected && http_should_keep_alive(&t->parser))
 		return GIT_SUCCESS;
 
-	s = gitno_connect(host, port);
-	if (s < GIT_SUCCESS) {
-	    return git__rethrow(s, "Failed to connect to host");
+	error = gitno_connect(host, port, &s);
+	if (error != GIT_SUCCESS) {
+	    return git__rethrow(error, "Failed to connect to host");
 	}
 	t->socket = s;
 	t->parent.connected = 1;