From de069b99774221193530f9eaa43ebfa417be941f Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 5 Sep 2008 16:29:56 +0000 Subject: [PATCH] On win32, errno is not the last socket error. Worse, WSAGetLastError() is not the last socket error sometimes (i.e., EWOULDBLOCK). Also, strerror() does not handle winsock errors. Therefore, event_err() and event_warn() are completely wrong for windows socket errors. Fix that. svn:r936 --- ChangeLog | 3 +- evdns.c | 34 +++++----------- event.c | 4 +- evutil.c | 94 +++++++++++++++++++++++++++++++++++++++++++ http.c | 14 +++---- include/event2/util.h | 14 +++++++ log.c | 44 +++++++++++++++----- log.h | 2 + signal.c | 10 ++++- 9 files changed, 171 insertions(+), 48 deletions(-) diff --git a/ChangeLog b/ChangeLog index aeb0882f..f751392e 100644 --- a/ChangeLog +++ b/ChangeLog @@ -123,7 +123,8 @@ Changes in current version: o Make event_add not change any state if it fails; reported by Ian Bell. o Fix a bug where headers arriving in multiple packets were not parsed; fix from Jiang Hong; test by me. o Match the query in DNS replies to the query in the request; from Vsevolod Stakhov. - + o Add new utility functions to correctly observe and log winsock errors. + Changes in 1.4.0: o allow \r or \n individually to separate HTTP headers instead of the standard "\r\n"; from Charles Kerr. o demote most http warnings to debug messages diff --git a/evdns.c b/evdns.c index 2325557c..8ce311e0 100644 --- a/evdns.c +++ b/evdns.c @@ -362,21 +362,6 @@ static int strtoint(const char *const str); #ifdef WIN32 static int -last_error(evutil_socket_t sock) -{ - int optval, optvallen=sizeof(optval); - int err = WSAGetLastError(); - if (err == WSAEWOULDBLOCK && sock >= 0) { - if (getsockopt(sock, SOL_SOCKET, SO_ERROR, (void*)&optval, - &optvallen)) - return err; - if (optval) - return optval; - } - return err; - -} -static int error_is_eagain(int err) { return err == EAGAIN || err == WSAEWOULDBLOCK; @@ -396,7 +381,6 @@ inet_aton(const char *c, struct in_addr *addr) return 1; } #else -#define last_error(sock) (errno) #define error_is_eagain(err) ((err) == EAGAIN) #endif #define CLOSE_SOCKET(s) EVUTIL_CLOSESOCKET(s) @@ -1183,9 +1167,9 @@ nameserver_read(struct nameserver *ns) { for (;;) { const int r = recv(ns->socket, packet, sizeof(packet), 0); if (r < 0) { - int err = last_error(ns->socket); + int err = evutil_socket_geterror(ns->socket); if (error_is_eagain(err)) return; - nameserver_failed(ns, strerror(err)); + nameserver_failed(ns, evutil_socket_error_to_string(err)); return; } ns->timedout = 0; @@ -1207,10 +1191,10 @@ server_port_read(struct evdns_server_port *s) { r = recvfrom(s->socket, packet, sizeof(packet), 0, (struct sockaddr*) &addr, &addrlen); if (r < 0) { - int err = last_error(s->socket); + int err = evutil_socket_geterror(s->socket); if (error_is_eagain(err)) return; log(EVDNS_LOG_WARN, "Error %s (%d) while reading request.", - strerror(err), err); + evutil_socket_error_to_string(err), err); return; } request_parse(packet, r, s, (struct sockaddr*) &addr, addrlen); @@ -1226,10 +1210,10 @@ server_port_flush(struct evdns_server_port *port) int r = sendto(port->socket, req->response, req->response_len, 0, (struct sockaddr*) &req->addr, req->addrlen); if (r < 0) { - int err = last_error(port->socket); + int err = evutil_socket_geterror(port->socket); if (error_is_eagain(err)) return; - log(EVDNS_LOG_WARN, "Error %s (%d) while writing response to port; dropping", strerror(err), err); + log(EVDNS_LOG_WARN, "Error %s (%d) while writing response to port; dropping", evutil_socket_error_to_string(err), err); } if (server_request_free(req)) { /* we released the last reference to req->port. */ @@ -1758,7 +1742,7 @@ evdns_server_request_respond(struct evdns_server_request *_req, int err) r = sendto(port->socket, req->response, req->response_len, 0, (struct sockaddr*) &req->addr, req->addrlen); if (r<0) { - int sock_err = last_error(port->socket); + int sock_err = evutil_socket_geterror(port->socket); if (! error_is_eagain(sock_err)) return -1; @@ -1936,9 +1920,9 @@ static int evdns_request_transmit_to(struct evdns_request *req, struct nameserver *server) { const int r = send(server->socket, req->request, req->request_len, 0); if (r < 0) { - int err = last_error(server->socket); + int err = evutil_socket_geterror(server->socket); if (error_is_eagain(err)) return 1; - nameserver_failed(req->ns, strerror(err)); + nameserver_failed(req->ns, evutil_socket_error_to_string(err)); return 2; } else if (r != (int)req->request_len) { return 1; /* short write */ diff --git a/event.c b/event.c index 77d2f1de..f01354a9 100644 --- a/event.c +++ b/event.c @@ -1487,13 +1487,13 @@ evthread_set_id_callback(struct event_base *base, #endif base->th_get_id = id_fn; base->th_owner_id = (*id_fn)(); - /* + /* * If another thread wants to add a new event, we need to notify * the thread that owns the base to wakeup for rescheduling. */ if (evutil_socketpair(LOCAL_SOCKETPAIR_AF, SOCK_STREAM, 0, base->th_notify_fd) == -1) - event_err(1, "%s: socketpair", __func__); + event_sock_err(1, -1, "%s: socketpair", __func__); evutil_make_socket_nonblocking(base->th_notify_fd[0]); evutil_make_socket_nonblocking(base->th_notify_fd[1]); diff --git a/evutil.c b/evutil.c index d591956f..43f1227c 100644 --- a/evutil.c +++ b/evutil.c @@ -218,6 +218,100 @@ evutil_gettimeofday(struct timeval *tv, struct timezone *tz) } #endif +#ifdef WIN32 +int +evutil_socket_geterror(evutil_socket_t sock) +{ + int optval, optvallen=sizeof(optval); + int err = WSAGetLastError(); + if (err == WSAEWOULDBLOCK && sock >= 0) { + if (getsockopt(sock, SOL_SOCKET, SO_ERROR, (void*)&optval, + &optvallen)) + return err; + if (optval) + return optval; + } + return err; +} +#endif + +#ifdef WIN32 +#define E(code, s) { code, (s " [" #code " ]") } +static struct { int code; const char *msg; } windows_socket_errors[] = { + E(WSAEINTR, "Interrupted function call"), + E(WSAEACCES, "Permission denied"), + E(WSAEFAULT, "Bad address"), + E(WSAEINVAL, "Invalid argument"), + E(WSAEMFILE, "Too many open files"), + E(WSAEWOULDBLOCK, "Resource temporarily unavailable"), + E(WSAEINPROGRESS, "Operation now in progress"), + E(WSAEALREADY, "Operation already in progress"), + E(WSAENOTSOCK, "Socket operation on nonsocket"), + E(WSAEDESTADDRREQ, "Destination address required"), + E(WSAEMSGSIZE, "Message too long"), + E(WSAEPROTOTYPE, "Protocol wrong for socket"), + E(WSAENOPROTOOPT, "Bad protocol option"), + E(WSAEPROTONOSUPPORT, "Protocol not supported"), + E(WSAESOCKTNOSUPPORT, "Socket type not supported"), + /* What's the difference between NOTSUPP and NOSUPPORT? :) */ + E(WSAEOPNOTSUPP, "Operation not supported"), + E(WSAEPFNOSUPPORT, "Protocol family not supported"), + E(WSAEAFNOSUPPORT, "Address family not supported by protocol family"), + E(WSAEADDRINUSE, "Address already in use"), + E(WSAEADDRNOTAVAIL, "Cannot assign requested address"), + E(WSAENETDOWN, "Network is down"), + E(WSAENETUNREACH, "Network is unreachable"), + E(WSAENETRESET, "Network dropped connection on reset"), + E(WSAECONNABORTED, "Software caused connection abort"), + E(WSAECONNRESET, "Connection reset by peer"), + E(WSAENOBUFS, "No buffer space available"), + E(WSAEISCONN, "Socket is already connected"), + E(WSAENOTCONN, "Socket is not connected"), + E(WSAESHUTDOWN, "Cannot send after socket shutdown"), + E(WSAETIMEDOUT, "Connection timed out"), + E(WSAECONNREFUSED, "Connection refused"), + E(WSAEHOSTDOWN, "Host is down"), + E(WSAEHOSTUNREACH, "No route to host"), + E(WSAEPROCLIM, "Too many processes"), + + /* Yes, some of these start with WSA, not WSAE. No, I don't know why. */ + E(WSASYSNOTREADY, "Network subsystem is unavailable"), + E(WSAVERNOTSUPPORTED, "Winsock.dll out of range"), + E(WSANOTINITIALISED, "Successful WSAStartup not yet performed"), + E(WSAEDISCON, "Graceful shutdown now in progress"), +#ifdef WSATYPE_NOT_FOUND + E(WSATYPE_NOT_FOUND, "Class type not found"), +#endif + E(WSAHOST_NOT_FOUND, "Host not found"), + E(WSATRY_AGAIN, "Nonauthoritative host not found"), + E(WSANO_RECOVERY, "This is a nonrecoverable error"), + E(WSANO_DATA, "Valid name, no data record of requested type)"), + + /* There are some more error codes whose numeric values are marked + * OS dependent. They start with WSA_, apparently for the same + * reason that practitioners of some craft traditions deliberately + * introduce imperfections into their baskets and rugs "to allow the + * evil spirits to escape." If we catch them, then our binaries + * might not report consistent results across versions of Windows. + * Thus, I'm going to let them all fall through. + */ + { -1, NULL }, +}; +#undef E +/** Equivalent to strerror, but for windows socket errors. */ +const char * +evutil_socket_error_to_string(int errcode) +{ + /* XXXX Is there really no built-in function to do this? */ + int i; + for (i=0; windows_socket_errors[i].code >= 0; ++i) { + if (e == windows_socket_errors[i].code) + return windows_socket_errors[i].msg; + } + return strerror(e); +} +#endif + int evutil_snprintf(char *buf, size_t buflen, const char *format, ...) { diff --git a/http.c b/http.c index 0833002e..55c013e5 100644 --- a/http.c +++ b/http.c @@ -1141,7 +1141,7 @@ evhttp_connection_cb(struct bufferevent *bufev, void *arg) struct evhttp_connection *evcon = arg; int error; socklen_t errsz = sizeof(error); - + /* Check if the connection completed */ if (getsockopt(evcon->fd, SOL_SOCKET, SO_ERROR, (void*)&error, &errsz) == -1) { @@ -1153,7 +1153,7 @@ evhttp_connection_cb(struct bufferevent *bufev, void *arg) if (error) { event_debug(("%s: connect failed for \"%s:%d\" on %d: %s", __func__, evcon->address, evcon->port, evcon->fd, - strerror(error))); + evutil_socket_error_to_string(error))); goto cleanup; } @@ -1770,7 +1770,7 @@ evhttp_connection_connect(struct evhttp_connection *evcon) } if (socket_connect(evcon->fd, evcon->address, evcon->port) == -1) { - event_warn("%s: connection to \"%s\" failed", + event_sock_warn(evcon->fd, "%s: connection to \"%s\" failed", __func__, evcon->address); EVUTIL_CLOSESOCKET(evcon->fd); evcon->fd = -1; return (-1); @@ -2340,7 +2340,7 @@ evhttp_bind_socket(struct evhttp *http, const char *address, ev_uint16_t port) return (-1); if (listen(fd, 128) == -1) { - event_warn("%s: listen", __func__); + event_sock_warn(fd, "%s: listen", __func__); EVUTIL_CLOSESOCKET(fd); return (-1); } @@ -2763,7 +2763,7 @@ evhttp_get_request(struct evhttp *http, evutil_socket_t fd, evcon = evhttp_get_request_connection(http, fd, sa, salen); if (evcon == NULL) { - event_warn("%s: cannot get connection on %d", __func__, fd); + event_sock_warn(fd, "%s: cannot get connection on %d", __func__, fd); EVUTIL_CLOSESOCKET(fd); return; } @@ -2860,8 +2860,8 @@ bind_socket_ai(struct addrinfo *ai, int reuse) /* Create listen socket */ fd = socket(AF_INET, SOCK_STREAM, 0); if (fd == -1) { - event_warn("socket"); - return (-1); + event_sock_warn(-1, "socket"); + return (-1); } if (evutil_make_socket_nonblocking(fd) < 0) diff --git a/include/event2/util.h b/include/event2/util.h index c78c0870..e082b49c 100644 --- a/include/event2/util.h +++ b/include/event2/util.h @@ -117,14 +117,28 @@ int evutil_make_socket_nonblocking(evutil_socket_t sock); #define EVUTIL_CLOSESOCKET(s) close(s) #endif +/* Winsock handles socket errors differently from the rest of the world. + * Elsewhere, a socket error is like any other error and is stored in errno. + * But winsock functions require you to retrieve the error with a special + * function, and don't let you use strerror for the error codes. And handling + * EWOULD block is ... different. */ + #ifdef WIN32 +/** Return the most recent socket error. Not idempotent. */ #define EVUTIL_SOCKET_ERROR() WSAGetLastError() +/** Replace the most recent socket error with errcode */ #define EVUTIL_SET_SOCKET_ERROR(errcode) \ do { WSASetLastError(errcode); } while (0) +/** Return the most recent socket error to occur on sock. */ +int evutil_socket_geterror(evutil_socket_t sock); +/** Convert a socket error to a string. */ +const char *evutil_socket_error_to_string(int errcode); #else #define EVUTIL_SOCKET_ERROR() (errno) #define EVUTIL_SET_SOCKET_ERROR(errcode) \ do { errno = (errcode); } while (0) +#define evutil_socket_geterror(sock) (errno) +#define evutil_socket_error_to_string(errcode) (strerror(errcode)) #endif /* diff --git a/log.c b/log.c index f69d401e..c141dba6 100644 --- a/log.c +++ b/log.c @@ -62,7 +62,7 @@ #include "log.h" -static void _warn_helper(int severity, int log_errno, const char *fmt, +static void _warn_helper(int severity, const char *errstr, const char *fmt, va_list ap); static void event_log(int severity, const char *msg); @@ -72,7 +72,7 @@ event_err(int eval, const char *fmt, ...) va_list ap; va_start(ap, fmt); - _warn_helper(_EVENT_LOG_ERR, errno, fmt, ap); + _warn_helper(_EVENT_LOG_ERR, strerror(errno), fmt, ap); va_end(ap); exit(eval); } @@ -83,7 +83,30 @@ event_warn(const char *fmt, ...) va_list ap; va_start(ap, fmt); - _warn_helper(_EVENT_LOG_WARN, errno, fmt, ap); + _warn_helper(_EVENT_LOG_WARN, strerror(errno), fmt, ap); + va_end(ap); +} + +void +event_sock_err(int eval, evutil_socket_t sock, const char *fmt, ...) +{ + va_list ap; + int err = evutil_socket_geterror(sock); + + va_start(ap, fmt); + _warn_helper(_EVENT_LOG_ERR, evutil_socket_error_to_string(err), fmt, ap); + va_end(ap); + exit(eval); +} + +void +event_sock_warn(int sock, const char *fmt, ...) +{ + va_list ap; + int err = evutil_socket_geterror(sock); + + va_start(ap, fmt); + _warn_helper(_EVENT_LOG_WARN, evutil_socket_error_to_string(err), fmt, ap); va_end(ap); } @@ -93,7 +116,7 @@ event_errx(int eval, const char *fmt, ...) va_list ap; va_start(ap, fmt); - _warn_helper(_EVENT_LOG_ERR, -1, fmt, ap); + _warn_helper(_EVENT_LOG_ERR, NULL, fmt, ap); va_end(ap); exit(eval); } @@ -104,7 +127,7 @@ event_warnx(const char *fmt, ...) va_list ap; va_start(ap, fmt); - _warn_helper(_EVENT_LOG_WARN, -1, fmt, ap); + _warn_helper(_EVENT_LOG_WARN, NULL, fmt, ap); va_end(ap); } @@ -114,7 +137,7 @@ event_msgx(const char *fmt, ...) va_list ap; va_start(ap, fmt); - _warn_helper(_EVENT_LOG_MSG, -1, fmt, ap); + _warn_helper(_EVENT_LOG_MSG, NULL, fmt, ap); va_end(ap); } @@ -124,12 +147,12 @@ _event_debugx(const char *fmt, ...) va_list ap; va_start(ap, fmt); - _warn_helper(_EVENT_LOG_DEBUG, -1, fmt, ap); + _warn_helper(_EVENT_LOG_DEBUG, NULL, fmt, ap); va_end(ap); } static void -_warn_helper(int severity, int log_errno, const char *fmt, va_list ap) +_warn_helper(int severity, const char *errstr, const char *fmt, va_list ap) { char buf[1024]; size_t len; @@ -139,11 +162,10 @@ _warn_helper(int severity, int log_errno, const char *fmt, va_list ap) else buf[0] = '\0'; - if (log_errno >= 0) { + if (errstr) { len = strlen(buf); if (len < sizeof(buf) - 3) { - evutil_snprintf(buf + len, sizeof(buf) - len, ": %s", - strerror(log_errno)); + evutil_snprintf(buf + len, sizeof(buf) - len, ": %s", errstr); } } diff --git a/log.h b/log.h index 7bc6632b..b537494e 100644 --- a/log.h +++ b/log.h @@ -35,6 +35,8 @@ void event_err(int eval, const char *fmt, ...) EV_CHECK_FMT(2,3); void event_warn(const char *fmt, ...) EV_CHECK_FMT(1,2); +void event_sock_err(int eval, int sock, const char *fmt, ...) EV_CHECK_FMT(3,4); +void event_sock_warn(int sock, const char *fmt, ...) EV_CHECK_FMT(2,3); void event_errx(int eval, const char *fmt, ...) EV_CHECK_FMT(2,3); void event_warnx(const char *fmt, ...) EV_CHECK_FMT(1,2); void event_msgx(const char *fmt, ...) EV_CHECK_FMT(1,2); diff --git a/signal.c b/signal.c index a5a329eb..64d96e28 100644 --- a/signal.c +++ b/signal.c @@ -82,7 +82,7 @@ evsignal_cb(evutil_socket_t fd, short what, void *arg) n = recv(fd, signals, sizeof(signals), 0); if (n == -1) - event_err(1, "%s: read", __func__); + event_sock_err(1, fd, "%s: read", __func__); } #ifdef HAVE_SETFD @@ -106,7 +106,7 @@ evsignal_init(struct event_base *base) */ if (evutil_socketpair( AF_UNIX, SOCK_STREAM, 0, base->sig.ev_signal_pair) == -1) - event_err(1, "%s: socketpair", __func__); + event_sock_err(1, -1, "%s: socketpair", __func__); FD_CLOSEONEXEC(base->sig.ev_signal_pair[0]); FD_CLOSEONEXEC(base->sig.ev_signal_pair[1]); @@ -278,6 +278,9 @@ static void evsignal_handler(int sig) { int save_errno = errno; +#ifdef WIN32 + int socket_errno = EVUTIL_SOCKET_ERROR(); +#endif if (evsignal_base == NULL) { event_warn( @@ -296,6 +299,9 @@ evsignal_handler(int sig) /* Wake up our notification mechanism */ send(evsignal_base->sig.ev_signal_pair[0], "a", 1, 0); errno = save_errno; +#ifdef WIN32 + EVUTIL_SET_SOCKET_ERRNO(socket_error); +#endif } void -- 2.50.1