From 7c11e51e1ab19e1eebc7f03d1a7a51f54acf7c32 Mon Sep 17 00:00:00 2001 From: Harlan Stenn Date: Sat, 27 Aug 2011 05:48:11 -0400 Subject: [PATCH 1/3] Clean up some problems identified by Coverity. --- bufferevent_sock.c | 5 +++-- evrpc.c | 11 +++++++++-- evutil.c | 2 +- sample/dns-example.c | 4 ++++ sample/http-server.c | 5 ++++- test/bench_http.c | 13 +++++++++++-- 6 files changed, 32 insertions(+), 8 deletions(-) diff --git a/bufferevent_sock.c b/bufferevent_sock.c index 6aff91ea..975b5b6c 100644 --- a/bufferevent_sock.c +++ b/bufferevent_sock.c @@ -113,8 +113,9 @@ bufferevent_socket_outbuf_cb(struct evbuffer *buf, !bufev_p->write_suspended) { /* Somebody added data to the buffer, and we would like to * write, and we were not writing. So, start writing. */ - be_socket_add(&bufev->ev_write, &bufev->timeout_write); - /* XXXX handle failure from be_socket_add */ + if (be_socket_add(&bufev->ev_write, &bufev->timeout_write) == -1) { + // Should we log this? + } } } diff --git a/evrpc.c b/evrpc.c index e11892dc..0c084a4a 100644 --- a/evrpc.c +++ b/evrpc.c @@ -338,6 +338,7 @@ static void evrpc_request_cb_closure(void *arg, enum EVRPC_HOOK_RESULT hook_res) { struct evrpc_req_generic *rpc_state = arg; + EVUTIL_ASSERT(rpc_state); struct evrpc *rpc = rpc_state->rpc; struct evhttp_request *req = rpc_state->http_req; @@ -399,8 +400,13 @@ evrpc_request_done_closure(void *, enum EVRPC_HOOK_RESULT); void evrpc_request_done(struct evrpc_req_generic *rpc_state) { - struct evhttp_request *req = rpc_state->http_req; - struct evrpc *rpc = rpc_state->rpc; + struct evhttp_request *req; + struct evrpc *rpc; + + EVUTIL_ASSERT(rpc_state); + + req = rpc_state->http_req; + rpc = rpc_state->rpc; if (rpc->reply_complete(rpc_state->reply) == -1) { /* the reply was not completely filled in. error out */ @@ -466,6 +472,7 @@ static void evrpc_request_done_closure(void *arg, enum EVRPC_HOOK_RESULT hook_res) { struct evrpc_req_generic *rpc_state = arg; + EVUTIL_ASSERT(rpc_state); struct evhttp_request *req = rpc_state->http_req; if (hook_res == EVRPC_TERMINATE) diff --git a/evutil.c b/evutil.c index adfcb6e5..f9d0c8b0 100644 --- a/evutil.c +++ b/evutil.c @@ -445,9 +445,9 @@ evutil_socket_connect(evutil_socket_t *fd_ptr, struct sockaddr *sa, int socklen) int made_fd = 0; if (*fd_ptr < 0) { - made_fd = 1; if ((*fd_ptr = socket(sa->sa_family, SOCK_STREAM, 0)) < 0) goto err; + made_fd = 1; if (evutil_make_socket_nonblocking(*fd_ptr) < 0) { goto err; } diff --git a/sample/dns-example.c b/sample/dns-example.c index f2c1e020..c5f377d3 100644 --- a/sample/dns-example.c +++ b/sample/dns-example.c @@ -173,6 +173,10 @@ main(int c, char **v) { evutil_socket_t sock; struct sockaddr_in my_addr; sock = socket(PF_INET, SOCK_DGRAM, 0); + if (sock == -1) { + perror("socket"); + exit(1); + } evutil_make_socket_nonblocking(sock); my_addr.sin_family = AF_INET; my_addr.sin_port = htons(10053); diff --git a/sample/http-server.c b/sample/http-server.c index 75caa414..cd6b27af 100644 --- a/sample/http-server.c +++ b/sample/http-server.c @@ -133,7 +133,8 @@ dump_request_cb(struct evhttp_request *req, void *arg) int n; char cbuf[128]; n = evbuffer_remove(buf, cbuf, sizeof(buf)-1); - fwrite(cbuf, 1, n, stdout); + if (n > 0) + fwrite(cbuf, 1, n, stdout); } puts(">>>"); @@ -179,6 +180,8 @@ send_document_cb(struct evhttp_request *req, void *arg) /* We need to decode it, to see what path the user really wanted. */ decoded_path = evhttp_uridecode(path, 0, NULL); + if (decoded_path == NULL) + goto err; /* Don't allow any ".."s in the path, to avoid exposing stuff outside * of the docroot. This test is both overzealous and underzealous: * it forbids aceptable paths like "/this/one..here", but it doesn't diff --git a/test/bench_http.c b/test/bench_http.c index dd3dde89..9fdd5aab 100644 --- a/test/bench_http.c +++ b/test/bench_http.c @@ -112,11 +112,20 @@ main(int argc, char **argv) } switch (c) { + char ** eptr; case 'p': - port = atoi(argv[i+1]); + port = (int)strtol(argv[i+1], eptr, 10); + if (!(argv[i+1] && (**eptr == '\0'))) { + fprintf(stderr, "Bad port\n"); + exit(1); + } break; case 'l': - content_len = atol(argv[i+1]); + content_len = (int)strtol(argv[i+1], eptr, 10); + if (!(argv[i+1] && (**eptr == '\0'))) { + fprintf(stderr, "Bad content length\n"); + exit(1); + } if (content_len == 0) { fprintf(stderr, "Bad content length\n"); exit(1); From 6056d6e0df8900a9e8878cb8321f1f33c04de187 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sun, 28 Aug 2011 14:02:40 -0400 Subject: [PATCH 2/3] Cleanup on 7c11e51e1ab: restore c90 declaration compliance --- evrpc.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/evrpc.c b/evrpc.c index 0c084a4a..72f08a89 100644 --- a/evrpc.c +++ b/evrpc.c @@ -338,9 +338,12 @@ static void evrpc_request_cb_closure(void *arg, enum EVRPC_HOOK_RESULT hook_res) { struct evrpc_req_generic *rpc_state = arg; + struct evrpc *rpc; + struct evhttp_request *req; + EVUTIL_ASSERT(rpc_state); - struct evrpc *rpc = rpc_state->rpc; - struct evhttp_request *req = rpc_state->http_req; + rpc = rpc_state->rpc; + req = rpc_state->http_req; if (hook_res == EVRPC_TERMINATE) goto error; @@ -472,8 +475,9 @@ static void evrpc_request_done_closure(void *arg, enum EVRPC_HOOK_RESULT hook_res) { struct evrpc_req_generic *rpc_state = arg; + struct evhttp_request *req; EVUTIL_ASSERT(rpc_state); - struct evhttp_request *req = rpc_state->http_req; + req = rpc_state->http_req; if (hook_res == EVRPC_TERMINATE) goto error; From 2f51dc0311566fab4f0a4b820ac8120fa803499a Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sun, 28 Aug 2011 14:03:10 -0400 Subject: [PATCH 3/3] Cleanup on 7c11e51e1ab: fix strtol usage --- test/bench_http.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/test/bench_http.c b/test/bench_http.c index 9fdd5aab..8180b267 100644 --- a/test/bench_http.c +++ b/test/bench_http.c @@ -91,6 +91,7 @@ main(int argc, char **argv) int c; int use_iocp = 0; unsigned short port = 8080; + char *endptr = NULL; #ifdef WIN32 WSADATA WSAData; @@ -112,21 +113,24 @@ main(int argc, char **argv) } switch (c) { - char ** eptr; case 'p': - port = (int)strtol(argv[i+1], eptr, 10); - if (!(argv[i+1] && (**eptr == '\0'))) { + if (i+1 >= argc || !argv[i+1]) { + fprintf(stderr, "Missing port\n"); + exit(1); + } + port = (int)strtol(argv[i+1], &endptr, 10); + if (*endptr != '\0') { fprintf(stderr, "Bad port\n"); exit(1); } break; case 'l': - content_len = (int)strtol(argv[i+1], eptr, 10); - if (!(argv[i+1] && (**eptr == '\0'))) { - fprintf(stderr, "Bad content length\n"); + if (i+1 >= argc || !argv[i+1]) { + fprintf(stderr, "Missing content length\n"); exit(1); } - if (content_len == 0) { + content_len = (size_t)strtol(argv[i+1], &endptr, 10); + if (*endptr != '\0' || content_len == 0) { fprintf(stderr, "Bad content length\n"); exit(1); }