From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: From: Vladislav Shpilevoy Subject: [PATCH 09/11] evio: refactoring Date: Fri, 30 Nov 2018 18:39:41 +0300 Message-Id: In-Reply-To: References: In-Reply-To: References: To: tarantool-patches@freelists.org Cc: vdavydov.dev@gmail.com List-ID: Remove useless, wrong and obvious comments. Correct code and comment style. Reuse some code, unnecessary try/catch. --- src/evio.cc | 186 ++++++++++++++++++++-------------------------------- src/evio.h | 40 ++++++----- 2 files changed, 89 insertions(+), 137 deletions(-) diff --git a/src/evio.cc b/src/evio.cc index 25f699bab..166ba7f95 100644 --- a/src/evio.cc +++ b/src/evio.cc @@ -29,7 +29,6 @@ * SUCH DAMAGE. */ #include "evio.h" -#include "uri.h" #include "scoped_guard.h" #include #include @@ -41,41 +40,33 @@ #include #include "exception.h" -static void -evio_setsockopt_server(int fd, int family, int type); - -/** Note: this function does not throw. */ void evio_close(ev_loop *loop, struct ev_io *evio) { /* Stop I/O events. Safe to do even if not started. */ ev_io_stop(loop, evio); - /* Close the socket. */ - close(evio->fd); - /* Make sure evio_has_fd() returns a proper value. */ - evio->fd = -1; + if (evio_has_fd(evio)) { + close(evio->fd); + ev_io_set(evio, -1, 0); + } } -/** - * Create an endpoint for communication. - * Set socket as non-block and apply protocol specific options. - */ void -evio_socket(struct ev_io *coio, int domain, int type, int protocol) +evio_socket(struct ev_io *evio, int domain, int type, int protocol) { - assert(coio->fd == -1); - coio->fd = sio_socket(domain, type, protocol); - if (coio->fd < 0) + assert(! evio_has_fd(evio)); + evio->fd = sio_socket(domain, type, protocol); + if (evio->fd < 0) diag_raise(); - if (evio_setsockopt_client(coio->fd, domain, type) != 0) { - close(coio->fd); - coio->fd = -1; + if (evio_setsockopt_client(evio->fd, domain, type) != 0) { + close(evio->fd); + ev_io_set(evio, -1, 0); diag_raise(); } } static int -evio_setsockopt_keepalive(int fd) +evio_setsockopt_keeping(int fd) { int on = 1; /* @@ -94,8 +85,8 @@ evio_setsockopt_keepalive(int fd) if (sio_setsockopt(fd, IPPROTO_TCP, TCP_KEEPCNT, &keepcnt, sizeof(int)) != 0) return -1; - int keepidle = 30; + int keepidle = 30; if (sio_setsockopt(fd, IPPROTO_TCP, TCP_KEEPIDLE, &keepidle, sizeof(int)) != 0) return -1; @@ -108,7 +99,6 @@ evio_setsockopt_keepalive(int fd) return 0; } -/** Set common client socket options. */ int evio_setsockopt_client(int fd, int family, int type) { @@ -116,11 +106,7 @@ evio_setsockopt_client(int fd, int family, int type) if (sio_setfl(fd, O_NONBLOCK) != 0) return -1; if (type == SOCK_STREAM && family != AF_UNIX) { - /* - * SO_KEEPALIVE to ensure connections don't hang - * around for too long when a link goes away. - */ - if (evio_setsockopt_keepalive(fd) != 0) + if (evio_setsockopt_keeping(fd) != 0) return -1; /* * Lower latency is more important than higher @@ -139,81 +125,63 @@ static void evio_setsockopt_server(int fd, int family, int type) { int on = 1; - /* In case this throws, the socket is not leaked. */ if (sio_setfl(fd, O_NONBLOCK) != 0) diag_raise(); - /* Allow reuse local adresses. */ - if (sio_setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, - &on, sizeof(on))) + if (sio_setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &on, sizeof(on)) != 0) diag_raise(); - /* Send all buffered messages on socket before take - * control out from close(2) or shutdown(2). */ + /* + * Send all buffered messages on socket before take + * control out from close() or shutdown(). + */ struct linger linger = { 0, 0 }; - - if (sio_setsockopt(fd, SOL_SOCKET, SO_LINGER, - &linger, sizeof(linger))) + if (sio_setsockopt(fd, SOL_SOCKET, SO_LINGER, &linger, + sizeof(linger)) != 0) diag_raise(); if (type == SOCK_STREAM && family != AF_UNIX && - evio_setsockopt_keepalive(fd) != 0) + evio_setsockopt_keeping(fd) != 0) diag_raise(); } -static inline const char * -evio_service_name(struct evio_service *service) -{ - return service->name; -} - /** * A callback invoked by libev when acceptor socket is ready. * Accept the socket, initialize it and pass to the on_accept * callback. */ static void -evio_service_accept_cb(ev_loop * /* loop */, ev_io *watcher, - int /* revents */) +evio_service_accept_cb(ev_loop *loop, ev_io *watcher, int events) { + (void) loop; + (void) events; struct evio_service *service = (struct evio_service *) watcher->data; - + int fd; while (1) { /* - * Accept all pending connections from backlog during event - * loop iteration. Significally speed up acceptor with enabled + * Accept all pending connections from backlog + * during event loop iteration. Significally + * speed up acceptor with enabled * io_collect_interval. */ - int fd = -1; - try { - struct sockaddr_storage addr; - socklen_t addrlen = sizeof(addr); - bool is_error_fatal; - fd = sio_accept(service->ev.fd, - (struct sockaddr *)&addr, &addrlen, - &is_error_fatal); - if (fd < 0) { - if (is_error_fatal) - diag_raise(); - return; - } - /* set common client socket options */ - if (evio_setsockopt_client(fd, service->addr.sa_family, - SOCK_STREAM) != 0) - diag_raise(); - /* - * Invoke the callback and pass it the accepted - * socket. - */ - if (service->on_accept(service, fd, - (struct sockaddr *)&addr, - addrlen) != 0) - diag_raise(); - } catch (Exception *e) { - if (fd >= 0) - close(fd); - e->log(); + struct sockaddr_storage addr; + socklen_t addrlen = sizeof(addr); + bool is_error_fatal; + fd = sio_accept(service->ev.fd, (struct sockaddr *)&addr, + &addrlen, &is_error_fatal); + if (fd < 0) { + if (is_error_fatal) + break; return; } + if (evio_setsockopt_client(fd, service->addr.sa_family, + SOCK_STREAM) != 0) + break; + if (service->on_accept(service, fd, (struct sockaddr *)&addr, + addrlen) != 0) + break; } + diag_log(); + if (fd >= 0) + close(fd); } /* @@ -240,7 +208,7 @@ evio_service_reuse_addr(struct evio_service *service, int fd) if (errno != ECONNREFUSED) goto err; - if (unlink(((struct sockaddr_un *)(&service->addr))->sun_path)) + if (unlink(((struct sockaddr_un *)(&service->addr))->sun_path) != 0) goto err; close(cl_fd); @@ -260,11 +228,10 @@ err: static void evio_service_bind_addr(struct evio_service *service) { - say_debug("%s: binding to %s...", evio_service_name(service), + say_debug("%s: binding to %s...", service->name, sio_strfaddr(&service->addr, service->addr_len)); /* Create a socket. */ - int fd = sio_socket(service->addr.sa_family, - SOCK_STREAM, IPPROTO_TCP); + int fd = sio_socket(service->addr.sa_family, SOCK_STREAM, IPPROTO_TCP); if (fd < 0) diag_raise(); @@ -286,7 +253,7 @@ evio_service_bind_addr(struct evio_service *service) } } - say_info("%s: bound to %s", evio_service_name(service), + say_info("%s: bound to %s", service->name, sio_strfaddr(&service->addr, service->addr_len)); /* Register the socket in the event loop. */ @@ -295,15 +262,10 @@ evio_service_bind_addr(struct evio_service *service) fd_guard.is_active = false; } -/** - * Listen on bounded port. - * - * @retval 0 for success - */ void evio_service_listen(struct evio_service *service) { - say_debug("%s: listening on %s...", evio_service_name(service), + say_debug("%s: listening on %s...", service->name, sio_strfaddr(&service->addr, service->addr_len)); int fd = service->ev.fd; @@ -332,28 +294,25 @@ evio_service_init(ev_loop *loop, struct evio_service *service, const char *name, service->ev.data = service; } -/** - * Try to bind. - */ void evio_service_bind(struct evio_service *service, const char *uri) { struct uri u; - if (uri_parse(&u, uri) || u.service == NULL) + if (uri_parse(&u, uri) || u.service == NULL) { tnt_raise(SocketError, sio_socketname(-1), "invalid uri for bind: %s", uri); + } snprintf(service->serv, sizeof(service->serv), "%.*s", (int) u.service_len, u.service); if (u.host != NULL && strncmp(u.host, "*", u.host_len) != 0) { snprintf(service->host, sizeof(service->host), "%.*s", - (int) u.host_len, u.host); - } /* else { service->host[0] = '\0'; } */ + (int) u.host_len, u.host); + } assert(! ev_is_active(&service->ev)); if (strcmp(service->host, URI_HOST_UNIX) == 0) { - /* UNIX domain socket */ struct sockaddr_un *un = (struct sockaddr_un *) &service->addr; service->addr_len = sizeof(*un); snprintf(un->sun_path, sizeof(un->sun_path), "%s", @@ -362,18 +321,22 @@ evio_service_bind(struct evio_service *service, const char *uri) return evio_service_bind_addr(service); } - /* IP socket */ + /* IP socket. */ struct addrinfo hints, *res; memset(&hints, 0, sizeof(hints)); hints.ai_family = AF_UNSPEC; hints.ai_socktype = SOCK_STREAM; hints.ai_flags = AI_PASSIVE|AI_ADDRCONFIG; - /* make no difference between empty string and NULL for host */ + /* + * Make no difference between empty string and NULL for + * host. + */ if (getaddrinfo(*service->host ? service->host : NULL, service->serv, - &hints, &res) != 0 || res == NULL) + &hints, &res) != 0 || res == NULL) { tnt_raise(SocketError, sio_socketname(-1), "can't resolve uri for bind"); + } auto addrinfo_guard = make_scoped_guard([=]{ freeaddrinfo(res); }); for (struct addrinfo *ai = res; ai != NULL; ai = ai->ai_next) { @@ -382,32 +345,23 @@ evio_service_bind(struct evio_service *service, const char *uri) try { return evio_service_bind_addr(service); } catch (SocketError *e) { - say_error("%s: failed to bind on %s: %s", - evio_service_name(service), + say_error("%s: failed to bind on %s: %s", service->name, sio_strfaddr(ai->ai_addr, ai->ai_addrlen), e->get_errmsg()); /* ignore */ } } tnt_raise(SocketError, sio_socketname(-1), "%s: failed to bind", - evio_service_name(service)); + service->name); } -/** It's safe to stop a service which is not started yet. */ void evio_service_stop(struct evio_service *service) { - say_info("%s: stopped", evio_service_name(service)); - - if (ev_is_active(&service->ev)) { - ev_io_stop(service->loop, &service->ev); - } - - if (service->ev.fd >= 0) { - close(service->ev.fd); - ev_io_set(&service->ev, -1, 0); - if (service->addr.sa_family == AF_UNIX) { - unlink(((struct sockaddr_un *) &service->addr)->sun_path); - } - } + say_info("%s: stopped", service->name); + bool unlink_unix = evio_has_fd(&service->ev) && + service->addr.sa_family == AF_UNIX; + evio_close(service->loop, &service->ev); + if (unlink_unix) + unlink(((struct sockaddr_un *) &service->addr)->sun_path); } diff --git a/src/evio.h b/src/evio.h index 9f80e84a5..ae2b9b9a8 100644 --- a/src/evio.h +++ b/src/evio.h @@ -39,15 +39,14 @@ #include "sio.h" #include "uri.h" /** - * Exception-aware way to add a listening socket to the event - * loop. Callbacks are invoked on bind and accept events. + * Exception-aware way to add a socket to the event loop. * - * Coroutines/fibers are not used for port listeners - * since listener's job is usually simple and only involves - * creating a session for the accepted socket. The session itself - * can be built around simple libev callbacks, or around - * cooperative multitasking (on_accept callback can create - * a fiber and use coio.h (cooperative multi-tasking I/O)) API. + * Coroutines/fibers are not used for port listeners since + * listener's job is usually simple and only involves creating a + * session for the accepted socket. The session itself can be + * built around simple libev callbacks, or around cooperative + * multitasking (on_accept callback can create a fiber and use + * coio.h (cooperative multi-tasking I/O)) API. * * How to use a service: * struct evio_service *service; @@ -58,9 +57,6 @@ * ... * evio_service_stop(service); * free(service); - * - * If a service is not started, but only initialized, no - * dedicated cleanup/destruction is necessary. */ struct evio_service; @@ -101,15 +97,11 @@ void evio_service_init(ev_loop *loop, struct evio_service *service, const char *name, evio_accept_f on_accept, void *on_accept_param); -/** Bind service to specified uri */ +/** Bind service to specified uri. */ void evio_service_bind(struct evio_service *service, const char *uri); -/** - * Listen on bounded socket - * - * @retval 0 for success - */ +/** Listen on bounded socket. */ void evio_service_listen(struct evio_service *service); @@ -117,22 +109,27 @@ evio_service_listen(struct evio_service *service); void evio_service_stop(struct evio_service *service); +/** + * Create a client socket. Sets keepalive, nonblock and nodelay + * options. + */ void evio_socket(struct ev_io *coio, int domain, int type, int protocol); +/** Close evio service socket and detach from event loop. */ void evio_close(ev_loop *loop, struct ev_io *evio); static inline bool -evio_service_is_active(struct evio_service *service) +evio_has_fd(struct ev_io *ev) { - return service->ev.fd >= 0; + return ev->fd >= 0; } static inline bool -evio_has_fd(struct ev_io *ev) +evio_service_is_active(struct evio_service *service) { - return ev->fd >= 0; + return evio_has_fd(&service->ev); } static inline void @@ -150,6 +147,7 @@ evio_timeout_update(ev_loop *loop, ev_tstamp start, ev_tstamp *delay) *delay = (elapsed >= *delay) ? 0 : *delay - elapsed; } +/** Set nonblock, keepalive and nodelay options to socket. */ int evio_setsockopt_client(int fd, int family, int type); -- 2.17.2 (Apple Git-113)