Commit b4b96d56bf1d9f362441e5e650b023040a319e05
2012-05-05T13:30:33
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.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107
diff --git a/src/netops.c b/src/netops.c
index 4b307af..e1ab243 100644
--- a/src/netops.c
+++ b/src/netops.c
@@ -74,12 +74,11 @@ void gitno_consume_n(gitno_buffer *buf, size_t cons)
buf->offset -= cons;
}
-int gitno_connect(const char *host, const char *port)
+int gitno_connect(const char *host, const char *port, GIT_SOCKET *s)
{
struct addrinfo *info, *p;
struct addrinfo hints;
int ret, error = GIT_SUCCESS;
- GIT_SOCKET s;
memset(&hints, 0x0, sizeof(struct addrinfo));
hints.ai_family = AF_UNSPEC;
@@ -93,24 +92,25 @@ int gitno_connect(const char *host, const char *port)
}
for (p = info; p != NULL; p = p->ai_next) {
- s = socket(p->ai_family, p->ai_socktype, p->ai_protocol);
+ *s = socket(p->ai_family, p->ai_socktype, p->ai_protocol);
#ifdef GIT_WIN32
- if (s == INVALID_SOCKET) {
+ if (*s == INVALID_SOCKET) {
#else
- if (s < 0) {
+ if (*s < 0) {
#endif
error = GIT_EOSERR;
goto cleanup;
}
- ret = connect(s, p->ai_addr, p->ai_addrlen);
+ ret = connect(*s, p->ai_addr, p->ai_addrlen);
/* If we can't connect, try the next one */
if (ret < 0) {
+ close(*s);
continue;
}
/* Return the socket */
- error = s;
+ error = GIT_SUCCESS;
goto cleanup;
}
diff --git a/src/netops.h b/src/netops.h
index 01ad971..cef39d9 100644
--- a/src/netops.h
+++ b/src/netops.h
@@ -25,7 +25,7 @@ int gitno_recv(gitno_buffer *buf);
void gitno_consume(gitno_buffer *buf, const char *ptr);
void gitno_consume_n(gitno_buffer *buf, size_t cons);
-int gitno_connect(const char *host, const char *port);
+int gitno_connect(const char *host, const char *port, GIT_SOCKET *s);
int gitno_send(GIT_SOCKET s, const char *msg, size_t len, int flags);
int gitno_close(GIT_SOCKET s);
int gitno_send_chunk_size(int s, size_t len);
diff --git a/src/transports/git.c b/src/transports/git.c
index 88e7e81..b0acc7e 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 2842d08..323b561 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;