From d0347dc4bab492910bc80276051eea31d39b23e9 Mon Sep 17 00:00:00 2001 From: Nathan French Date: Mon, 22 May 2017 14:11:01 -0700 Subject: [PATCH] Issue#6: make evhtp_accept_socket conform to api - evhtp_accept_socket's function definition states that 0 on success, -1 on error. This was not the case. Also evhtp_accept_socket should not close the file descriptor as it is passed as an argument, not an owner. There should be no negative effects of this change of the documentation for the function was used :D --- evhtp.c | 43 +++++++++++++++++++++---------------------- evhtp.h | 3 +++ 2 files changed, 24 insertions(+), 22 deletions(-) diff --git a/evhtp.c b/evhtp.c index be46b3f..4924b18 100644 --- a/evhtp.c +++ b/evhtp.c @@ -3568,12 +3568,9 @@ int evhtp_accept_socket(evhtp_t * htp, evutil_socket_t sock, int backlog) { int on = 1; - int res = 0; int err = 1; - evhtp_assert(htp != NULL); - - if (sock == -1) + if (htp == NULL || sock == -1) { return -1; } @@ -3582,7 +3579,7 @@ evhtp_accept_socket(evhtp_t * htp, evutil_socket_t sock, int backlog) #if defined SO_REUSEPORT if (htp->enable_reuseport) { - if ((res = setsockopt(sock, SOL_SOCKET, SO_REUSEPORT, (void *)&on, sizeof(on)) != 0)) + if (setsockopt(sock, SOL_SOCKET, SO_REUSEPORT, (void *)&on, sizeof(on)) == -1) { break; } @@ -3592,7 +3589,7 @@ evhtp_accept_socket(evhtp_t * htp, evutil_socket_t sock, int backlog) #if defined TCP_NODELAY if (htp->enable_nodelay == 1) { - if ((res = setsockopt(sock, IPPROTO_TCP, TCP_NODELAY, (void *)&on, sizeof(on))) != 0) + if (setsockopt(sock, IPPROTO_TCP, TCP_NODELAY, (void *)&on, sizeof(on)) == -1) { break; } @@ -3602,7 +3599,7 @@ evhtp_accept_socket(evhtp_t * htp, evutil_socket_t sock, int backlog) #if defined TCP_DEFER_ACCEPT if (htp->enable_defer_accept == 1) { - if ((res = setsockopt(sock, IPPROTO_TCP, TCP_DEFER_ACCEPT, (void *)&on, sizeof(on))) != 0) + if (setsockopt(sock, IPPROTO_TCP, TCP_DEFER_ACCEPT, (void *)&on, sizeof(on)) == -1) { break; } @@ -3638,8 +3635,6 @@ evhtp_accept_socket(evhtp_t * htp, evutil_socket_t sock, int backlog) if (err == 1) { - evutil_closesocket(sock); - if (htp->server != NULL) { evhtp_safe_free(htp->server, evconnlistener_free); @@ -3648,8 +3643,7 @@ evhtp_accept_socket(evhtp_t * htp, evutil_socket_t sock, int backlog) return -1; } - - return res; + return 0; } /* evhtp_accept_socket */ int @@ -3664,6 +3658,7 @@ evhtp_bind_sockaddr(evhtp_t * htp, struct sockaddr * sa, size_t sin_len, int bac return -1; } + /* XXX: API's should not set signals */ #ifndef WIN32 signal(SIGPIPE, SIG_IGN); #endif @@ -3715,26 +3710,32 @@ evhtp_bind_sockaddr(evhtp_t * htp, struct sockaddr * sa, size_t sin_len, int bac return -1; } - return evhtp_accept_socket(htp, fd, backlog); + if (evhtp_accept_socket(htp, fd, backlog) == -1) + { + /* accept_socket() does not close the descriptor + * on error, but this function does. + */ + evutil_closesocket(fd); + + return -1; + } + + return 0; } /* evhtp_bind_sockaddr */ int evhtp_bind_socket(evhtp_t * htp, const char * baddr, uint16_t port, int backlog) { #ifndef NO_SYS_UN - struct sockaddr_un sun; + struct sockaddr_un sun = { 0 }; #endif - struct sockaddr_in6 sin6; - struct sockaddr_in sin; struct sockaddr * sa; + struct sockaddr_in6 sin6 = { 0 }; + struct sockaddr_in sin = { 0 }; size_t sin_len; - memset(&sin, 0, sizeof(sin)); - if (!strncmp(baddr, "ipv6:", 5)) { - memset(&sin6, 0, sizeof(sin6)); - baddr += 5; sin_len = sizeof(struct sockaddr_in6); sin6.sin6_port = htons(port); @@ -3752,8 +3753,6 @@ evhtp_bind_socket(evhtp_t * htp, const char * baddr, uint16_t port, int backlog) return -1; } - memset(&sun, 0, sizeof(sun)); - sin_len = sizeof(struct sockaddr_un); sun.sun_family = AF_UNIX; @@ -3771,7 +3770,6 @@ evhtp_bind_socket(evhtp_t * htp, const char * baddr, uint16_t port, int backlog) } sin_len = sizeof(struct sockaddr_in); - sin.sin_family = AF_INET; sin.sin_port = htons(port); sin.sin_addr.s_addr = inet_addr(baddr); @@ -5041,6 +5039,7 @@ evhtp_make_request(evhtp_connection_t * c, evhtp_request_t * r, evhtp_headers_for_each(r->headers_out, htp__create_headers_, obuf); evbuffer_add_reference(obuf, "\r\n", 2, NULL, NULL); + if (evbuffer_get_length(r->buffer_out)) { evbuffer_add_buffer(obuf, r->buffer_out); diff --git a/evhtp.h b/evhtp.h index e0d7a58..b696edf 100644 --- a/evhtp.h +++ b/evhtp.h @@ -845,6 +845,9 @@ EVHTP_EXPORT void evhtp_unbind_socket(evhtp_t * htp); * @brief create the listener plus setup various options with an already-bound * socket. * + * @note Since the file descriptor is passed to the function, it will not + * attempt to close it if an error occurs. + * * @param htp * @param sock * @param backlog