From 2b386acdb310394b7a227b27d6eff330e1f935ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Tue, 6 Mar 2012 10:47:18 +0100 Subject: [PATCH] error-handling: git transport --- src/transports/git.c | 257 +++++++++++++++++++++---------------------- 1 file changed, 125 insertions(+), 132 deletions(-) diff --git a/src/transports/git.c b/src/transports/git.c index befdec5ee..fd3ff5b32 100644 --- a/src/transports/git.c +++ b/src/transports/git.c @@ -49,8 +49,10 @@ static int gen_proto(git_buf *request, const char *cmd, const char *url) int len; delim = strchr(url, '/'); - if (delim == NULL) - return git__throw(GIT_EOBJCORRUPTED, "Failed to create proto-request: malformed URL"); + if (delim == NULL) { + giterr_set(GITERR_NET, "Malformed URL"); + return -1; + } repo = delim; @@ -80,7 +82,7 @@ static int send_request(GIT_SOCKET s, const char *cmd, const char *url) git_buf request = GIT_BUF_INIT; error = gen_proto(&request, cmd, url); - if (error < GIT_SUCCESS) + if (error < 0) goto cleanup; error = gitno_send(s, request.ptr, request.size, 0); @@ -105,9 +107,8 @@ static int do_connect(transport_git *t, const char *url) if (!git__prefixcmp(url, prefix)) url += strlen(prefix); - error = gitno_extract_host_and_port(&host, &port, url, GIT_DEFAULT_PORT); - if (error < GIT_SUCCESS) - return error; + if (gitno_extract_host_and_port(&host, &port, url, GIT_DEFAULT_PORT) < 0) + return -1; s = gitno_connect(host, port); connected = 1; @@ -119,8 +120,10 @@ static int do_connect(transport_git *t, const char *url) if (error < GIT_SUCCESS && s > 0) close(s); - if (!connected) - error = git__throw(GIT_EOSERR, "Failed to connect to any of the addresses"); + if (!connected) { + giterr_set(GITERR_NET, "Failed to connect to the host"); + return -1; + } return error; } @@ -131,33 +134,30 @@ static int do_connect(transport_git *t, const char *url) static int store_refs(transport_git *t) { gitno_buffer *buf = &t->buf; - int error = GIT_SUCCESS; + int ret = 0; while (1) { - error = gitno_recv(buf); - if (error < GIT_SUCCESS) - return git__rethrow(GIT_EOSERR, "Failed to receive data"); - if (error == GIT_SUCCESS) /* Orderly shutdown, so exit */ - return GIT_SUCCESS; + if ((ret = gitno_recv(buf)) < 0) + return -1; + if (ret == 0) /* Orderly shutdown, so exit */ + return 0; - error = git_protocol_store_refs(&t->proto, buf->data, buf->offset); - if (error == GIT_ESHORTBUFFER) { + ret = git_protocol_store_refs(&t->proto, buf->data, buf->offset); + if (ret == GIT_ESHORTBUFFER) { gitno_consume_n(buf, buf->len); continue; } - if (error < GIT_SUCCESS) - return git__rethrow(error, "Failed to store refs"); + if (ret < 0) + return ret; gitno_consume_n(buf, buf->offset); if (t->proto.flush) { /* No more refs */ t->proto.flush = 0; - return GIT_SUCCESS; + return 0; } } - - return error; } static int detect_caps(transport_git *t) @@ -170,7 +170,7 @@ static int detect_caps(transport_git *t) pkt = git_vector_get(refs, 0); /* No refs or capabilites, odd but not a problem */ if (pkt == NULL || pkt->capabilities == NULL) - return GIT_SUCCESS; + return 0; ptr = pkt->capabilities; while (ptr != NULL && *ptr != '\0') { @@ -187,7 +187,7 @@ static int detect_caps(transport_git *t) ptr = strchr(ptr, ' '); } - return GIT_SUCCESS; + return 0; } /* @@ -197,36 +197,33 @@ static int detect_caps(transport_git *t) static int git_connect(git_transport *transport, int direction) { transport_git *t = (transport_git *) transport; - int error = GIT_SUCCESS; - if (direction == GIT_DIR_PUSH) - return git__throw(GIT_EINVALIDARGS, "Pushing is not supported with the git protocol"); + if (direction == GIT_DIR_PUSH) { + giterr_set(GITERR_NET, "Pushing over git:// is not supported"); + return -1; + } t->parent.direction = direction; - error = git_vector_init(&t->refs, 16, NULL); - if (error < GIT_SUCCESS) - goto cleanup; + if (git_vector_init(&t->refs, 16, NULL) < 0) + return -1; /* Connect and ask for the refs */ - error = do_connect(t, transport->url); - if (error < GIT_SUCCESS) - return error; + if (do_connect(t, transport->url) < 0) + goto cleanup; gitno_buffer_setup(&t->buf, t->buff, sizeof(t->buff), t->socket); t->parent.connected = 1; - error = store_refs(t); - if (error < GIT_SUCCESS) - return error; + if (store_refs(t) < 0) + goto cleanup; - error = detect_caps(t); + if (detect_caps(t) < 0) + goto cleanup; + return 0; cleanup: - if (error < GIT_SUCCESS) { - git_vector_free(&t->refs); - } - - return error; + git_vector_free(&t->refs); + return -1; } static int git_ls(git_transport *transport, git_headlist_cb list_cb, void *opaque) @@ -244,12 +241,51 @@ static int git_ls(git_transport *transport, git_headlist_cb list_cb, void *opaqu pkt = (git_pkt_ref *)p; - if (list_cb(&pkt->head, opaque) < 0) - return git__throw(GIT_ERROR, - "The user callback returned an error code"); + if (list_cb(&pkt->head, opaque) < 0) { + giterr_set(GITERR_NET, "User callback returned error"); + return -1; + } } - return GIT_SUCCESS; + return 0; +} + +/* Wait until we get an ack from the */ +static int recv_pkt(gitno_buffer *buf) +{ + const char *ptr = buf->data, *line_end; + git_pkt *pkt; + int pkt_type, error; + + do { + /* Wait for max. 1 second */ + if ((error = gitno_select_in(buf, 1, 0)) < 0) { + return -1; + } else if (error == 0) { + /* + * Some servers don't respond immediately, so if this + * happens, we keep sending information until it + * answers. Pretend we received a NAK to convince higher + * layers to do so. + */ + return GIT_PKT_NAK; + } + + if ((error = gitno_recv(buf)) < 0) + return -1; + + error = git_pkt_parse_line(&pkt, ptr, &line_end, buf->offset); + if (error == GIT_ESHORTBUFFER) + continue; + if (error < 0) + return -1; + } while (error); + + gitno_consume(buf, line_end); + pkt_type = pkt->type; + git__free(pkt); + + return pkt_type; } static int git_negotiate_fetch(git_transport *transport, git_repository *repo, const git_vector *wants) @@ -263,19 +299,15 @@ static int git_negotiate_fetch(git_transport *transport, git_repository *repo, c unsigned int i; gitno_buffer *buf = &t->buf; - error = git_pkt_send_wants(wants, &t->caps, t->socket); - if (error < GIT_SUCCESS) - return git__rethrow(error, "Failed to send wants list"); + if (git_pkt_send_wants(wants, &t->caps, t->socket) < 0) + return -1; - error = git_reference_listall(&refs, repo, GIT_REF_LISTALL); - if (error < GIT_ERROR) - return git__rethrow(error, "Failed to list all references"); + if (git_reference_listall(&refs, repo, GIT_REF_LISTALL) < 0) + return -1; + + if (git_revwalk_new(&walk, repo) < 0) + return -1; - error = git_revwalk_new(&walk, repo); - if (error < GIT_ERROR) { - error = git__rethrow(error, "Failed to list all references"); - goto cleanup; - } git_revwalk_sorting(walk, GIT_SORT_TIME); for (i = 0; i < refs.count; ++i) { @@ -283,20 +315,15 @@ static int git_negotiate_fetch(git_transport *transport, git_repository *repo, c if (!git__prefixcmp(refs.strings[i], GIT_REFS_TAGS_DIR)) continue; - error = git_reference_lookup(&ref, repo, refs.strings[i]); - if (error < GIT_ERROR) { - error = git__rethrow(error, "Failed to lookup %s", refs.strings[i]); - goto cleanup; - } + if (git_reference_lookup(&ref, repo, refs.strings[i]) < 0) + goto on_error; if (git_reference_type(ref) == GIT_REF_SYMBOLIC) continue; - error = git_revwalk_push(walk, git_reference_oid(ref)); - if (error < GIT_ERROR) { - error = git__rethrow(error, "Failed to push %s", refs.strings[i]); - goto cleanup; - } + if ((error = git_revwalk_push(walk, git_reference_oid(ref))) < 0) + goto on_error; + } git_strarray_free(&refs); @@ -306,67 +333,38 @@ static int git_negotiate_fetch(git_transport *transport, git_repository *repo, c * every once in a while. */ i = 0; - while ((error = git_revwalk_next(&oid, walk)) == GIT_SUCCESS) { + while ((error = git_revwalk_next(&oid, walk)) == 0) { error = git_pkt_send_have(&oid, t->socket); i++; if (i % 20 == 0) { - const char *ptr = buf->data, *line_end; - git_pkt *pkt; + int pkt_type; + git_pkt_send_flush(t->socket); - while (1) { - /* Wait for max. 1 second */ - error = gitno_select_in(buf, 1, 0); - if (error < GIT_SUCCESS) { - error = git__throw(GIT_EOSERR, "Error in select"); - } else if (error == 0) { - /* - * Some servers don't respond immediately, so if this - * happens, we keep sending information until it - * answers. - */ - break; - } + pkt_type = recv_pkt(buf); - error = gitno_recv(buf); - if (error < GIT_SUCCESS) { - error = git__rethrow(error, "Error receiving data"); - goto cleanup; - } - error = git_pkt_parse_line(&pkt, ptr, &line_end, buf->offset); - if (error == GIT_ESHORTBUFFER) - continue; - if (error < GIT_SUCCESS) { - error = git__rethrow(error, "Failed to get answer"); - goto cleanup; - } - - gitno_consume(buf, line_end); - - if (pkt->type == GIT_PKT_ACK) { - git__free(pkt); - error = GIT_SUCCESS; - goto done; - } else if (pkt->type == GIT_PKT_NAK) { - git__free(pkt); - break; - } else { - error = git__throw(GIT_ERROR, "Got unexpected pkt type"); - goto cleanup; - } + if (pkt_type == GIT_PKT_ACK) { + break; + } else if (pkt_type == GIT_PKT_NAK) { + continue; + } else { + giterr_set(GITERR_NET, "Unexpected pkt type"); + goto on_error; } + } } - if (error == GIT_EREVWALKOVER) - error = GIT_SUCCESS; + if (error != GIT_EREVWALKOVER) + goto on_error; -done: git_pkt_send_flush(t->socket); git_pkt_send_done(t->socket); -cleanup: git_revwalk_free(walk); + return 0; - return error; +on_error: + git_revwalk_free(walk); + return -1; } static int git_send_flush(git_transport *transport) @@ -386,7 +384,7 @@ static int git_send_done(git_transport *transport) static int git_download_pack(char **out, git_transport *transport, git_repository *repo) { transport_git *t = (transport_git *) transport; - int error = GIT_SUCCESS; + int error = 0, read_bytes; gitno_buffer *buf = &t->buf; git_pkt *pkt; const char *line_end, *ptr; @@ -394,7 +392,7 @@ static int git_download_pack(char **out, git_transport *transport, git_repositor /* * For now, we ignore everything and wait for the pack */ - while (1) { + do { ptr = buf->data; /* Whilst we're searching for the pack */ while (1) { @@ -419,34 +417,29 @@ static int git_download_pack(char **out, git_transport *transport, git_repositor gitno_consume(buf, line_end); } - error = gitno_recv(buf); - if (error < GIT_SUCCESS) - return git__rethrow(GIT_EOSERR, "Failed to receive data"); - if (error == 0) { /* Orderly shutdown */ - return GIT_SUCCESS; - } + read_bytes = gitno_recv(buf); + } while (read_bytes); - } + return read_bytes; } static int git_close(git_transport *transport) { transport_git *t = (transport_git*) transport; - int error; /* Can't do anything if there's an error, so don't bother checking */ git_pkt_send_flush(t->socket); - error = gitno_close(t->socket); - - if (error < 0) - error = git__throw(GIT_EOSERR, "Failed to close socket"); + if (gitno_close(t->socket) < 0) { + giterr_set(GITERR_NET, "Failed to close socket"); + return -1; + } #ifdef GIT_WIN32 WSACleanup(); #endif - return error; + return 0; } static void git_free(git_transport *transport) @@ -475,8 +468,7 @@ int git_transport_git(git_transport **out) #endif t = git__malloc(sizeof(transport_git)); - if (t == NULL) - return GIT_ENOMEM; + GITERR_CHECK_ALLOC(t); memset(t, 0x0, sizeof(transport_git)); @@ -497,9 +489,10 @@ int git_transport_git(git_transport **out) ret = WSAStartup(MAKEWORD(2,2), &t->wsd); if (ret != 0) { git_free(*out); - return git__throw(GIT_EOSERR, "Winsock init failed"); + giterr_set(GITERR_NET, "Winsock init failed"); + return -1; } #endif - return GIT_SUCCESS; + return 0; }