From d76556797ddeb6143fa9a1c597f3e72271927858 Mon Sep 17 00:00:00 2001 From: Bert Belder Date: Mon, 4 Dec 2017 00:11:24 +0100 Subject: [PATCH 1/9] doc: fix EPOLL_CTL_MOD -> EPOLL_CTL_DEL --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 54428d2..d1ced5f 100644 --- a/README.md +++ b/README.md @@ -88,7 +88,7 @@ int epoll_ctl(HANDLE ephnd, * `ephnd` must be a HANDLE created by `epoll_create` or `epoll_create1`. * `op` must be one of `EPOLL_CTL_ADD`, `EPOLL_CTL_MOD`, `EPOLL_CTL_DEL`. * It is recommended to always explicitly remove a socket from its epoll - set using `EPOLL_CTL_MOD` *before* closing it. As on Linux, sockets + set using `EPOLL_CTL_DEL` *before* closing it. As on Linux, sockets are automatically removed from the epoll set when they are closed, but wepoll may not be able to detect this until the next call to `epoll_wait()`. From 98fc483e76c1aac17deefda2d61bdfb0e597452a Mon Sep 17 00:00:00 2001 From: Bert Belder Date: Mon, 4 Dec 2017 17:20:58 +0100 Subject: [PATCH 2/9] test: remove unnecessary WS2tcpip.h includes --- test/test-auto-drop-on-close.c | 2 -- test/test-ctl-fuzz.c | 2 -- test/test-multi-poll.c | 2 -- test/test-oneshot-and-hangup.c | 2 -- 4 files changed, 8 deletions(-) diff --git a/test/test-auto-drop-on-close.c b/test/test-auto-drop-on-close.c index 02d8cf7..7b3cbb2 100644 --- a/test/test-auto-drop-on-close.c +++ b/test/test-auto-drop-on-close.c @@ -17,8 +17,6 @@ #include "wepoll.h" #include "win.h" -#include - static SOCKET create_and_add_socket(HANDLE ephnd, int events) { SOCKET sock; struct epoll_event ev; diff --git a/test/test-ctl-fuzz.c b/test/test-ctl-fuzz.c index 3382e32..3ba25ed 100644 --- a/test/test-ctl-fuzz.c +++ b/test/test-ctl-fuzz.c @@ -9,8 +9,6 @@ #include "wepoll.h" #include "win.h" -#include - #define NUM_SOCKETS 10000 #define RUN_TIME 5000 #define PRINT_INTERVAL 500 diff --git a/test/test-multi-poll.c b/test/test-multi-poll.c index 87e7890..cd68472 100644 --- a/test/test-multi-poll.c +++ b/test/test-multi-poll.c @@ -9,8 +9,6 @@ #include "wepoll.h" #include "win.h" -#include - #define PORT_COUNT 10 #define THREADS_PER_PORT 10 diff --git a/test/test-oneshot-and-hangup.c b/test/test-oneshot-and-hangup.c index 66384fb..725c080 100644 --- a/test/test-oneshot-and-hangup.c +++ b/test/test-oneshot-and-hangup.c @@ -6,8 +6,6 @@ #include "wepoll.h" #include "win.h" -#include - int tcp_socketpair(SOCKET socks[2]) { SOCKET listen_sock; struct sockaddr_in addr; From 981b3caf4eb055cfd0ec368d2b866c9a1318789d Mon Sep 17 00:00:00 2001 From: Bert Belder Date: Mon, 4 Dec 2017 20:04:30 +0100 Subject: [PATCH 3/9] api: epoll_create() and epoll_create1() should return NULL on failure --- README.md | 2 +- src/api.c | 7 ++++--- test/test-auto-drop-on-close.c | 2 +- test/test-multi-poll.c | 2 +- test/test-oneshot-and-hangup.c | 2 +- test/test-udp-pings.c | 2 +- 6 files changed, 9 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index d1ced5f..1165d3f 100644 --- a/README.md +++ b/README.md @@ -62,7 +62,7 @@ HANDLE epoll_create1(int flags); * Create a new epoll instance (port). * `size` is ignored but most be greater than zero. * `flags` must be zero as there are no supported flags. -* Returns `INVALID_HANDLE_VALUE` on failure. +* Returns `NULL` on failure. * [man page][man epoll_create] ### epoll_close diff --git a/src/api.c b/src/api.c index 5fefcc5..7f16269 100644 --- a/src/api.c +++ b/src/api.c @@ -35,8 +35,9 @@ static HANDLE _epoll_create(void) { if (reflock_tree_add(&_epoll_handle_tree, &port_info->handle_tree_node, (uintptr_t) ephnd) < 0) { + /* This should never happen. */ ep_port_delete(port_info); - return_error(INVALID_HANDLE_VALUE, ERROR_ALREADY_EXISTS); + return_error(NULL, ERROR_ALREADY_EXISTS); } return ephnd; @@ -44,14 +45,14 @@ static HANDLE _epoll_create(void) { HANDLE epoll_create(int size) { if (size <= 0) - return_error(INVALID_HANDLE_VALUE, ERROR_INVALID_PARAMETER); + return_error(NULL, ERROR_INVALID_PARAMETER); return _epoll_create(); } HANDLE epoll_create1(int flags) { if (flags != 0) - return_error(INVALID_HANDLE_VALUE, ERROR_INVALID_PARAMETER); + return_error(NULL, ERROR_INVALID_PARAMETER); return _epoll_create(); } diff --git a/test/test-auto-drop-on-close.c b/test/test-auto-drop-on-close.c index 7b3cbb2..df2a782 100644 --- a/test/test-auto-drop-on-close.c +++ b/test/test-auto-drop-on-close.c @@ -60,7 +60,7 @@ int main(void) { int r; ephnd = epoll_create1(0); - check(ephnd != INVALID_HANDLE_VALUE); + check(ephnd != NULL); sock1 = create_and_add_socket(ephnd, EPOLLIN); sock2 = create_and_add_socket(ephnd, EPOLLIN); diff --git a/test/test-multi-poll.c b/test/test-multi-poll.c index cd68472..643b982 100644 --- a/test/test-multi-poll.c +++ b/test/test-multi-poll.c @@ -107,7 +107,7 @@ int main(void) { /* Create epoll port. */ port = epoll_create1(0); - check(port != INVALID_HANDLE_VALUE); + check(port != NULL); ports[i] = port; /* Register recv_sock with the epoll port. */ diff --git a/test/test-oneshot-and-hangup.c b/test/test-oneshot-and-hangup.c index 725c080..ecf58c9 100644 --- a/test/test-oneshot-and-hangup.c +++ b/test/test-oneshot-and-hangup.c @@ -65,7 +65,7 @@ int main(void) { { /* Create an epoll instance. */ epoll_port = epoll_create(1); - check(epoll_port > 0); + check(epoll_port != NULL); } { diff --git a/test/test-udp-pings.c b/test/test-udp-pings.c index 428ed15..704e375 100644 --- a/test/test-udp-pings.c +++ b/test/test-udp-pings.c @@ -24,7 +24,7 @@ int main(void) { struct epoll_event ev; epoll_hnd = epoll_create1(0); - check(epoll_hnd && epoll_hnd != INVALID_HANDLE_VALUE); + check(epoll_hnd != NULL); srv = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP); r = ioctlsocket(srv, FIONBIO, &one); From 69a297fd22f8d69b3033e20760c68ad1cacc80bd Mon Sep 17 00:00:00 2001 From: Bert Belder Date: Mon, 4 Dec 2017 20:06:16 +0100 Subject: [PATCH 4/9] port: properly propagate CreateIoCompletionPort() error --- src/port.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/port.c b/src/port.c index 38228a5..16fbea0 100644 --- a/src/port.c +++ b/src/port.c @@ -27,6 +27,14 @@ static void _ep_port_free(ep_port_t* port) { free(port); } +static HANDLE _ep_port_create_iocp(void) { + HANDLE iocp = CreateIoCompletionPort(INVALID_HANDLE_VALUE, NULL, 0, 0); + if (iocp == NULL) + return_error(NULL); + + return iocp; +} + ep_port_t* ep_port_new(HANDLE* iocp_out) { ep_port_t* port_info; HANDLE iocp; @@ -35,8 +43,8 @@ ep_port_t* ep_port_new(HANDLE* iocp_out) { if (port_info == NULL) goto err1; - iocp = CreateIoCompletionPort(INVALID_HANDLE_VALUE, NULL, 0, 0); - if (iocp == INVALID_HANDLE_VALUE) + iocp = _ep_port_create_iocp(); + if (iocp == NULL) goto err2; memset(port_info, 0, sizeof *port_info); From dc1369886d0626ad1432e8f01bcb50d4d119bd25 Mon Sep 17 00:00:00 2001 From: Bert Belder Date: Mon, 4 Dec 2017 20:06:39 +0100 Subject: [PATCH 5/9] error: map ERROR_*_SYSTEM_RESOURCES to ENOMEM --- src/error.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/error.c b/src/error.c index b99f40c..cd45cd5 100644 --- a/src/error.c +++ b/src/error.c @@ -44,6 +44,7 @@ X(ERROR_NETWORK_BUSY, ENETDOWN) \ X(ERROR_NETWORK_UNREACHABLE, ENETUNREACH) \ X(ERROR_NOACCESS, EFAULT) \ + X(ERROR_NONPAGED_SYSTEM_RESOURCES, ENOMEM) \ X(ERROR_NOT_ENOUGH_MEMORY, ENOMEM) \ X(ERROR_NOT_ENOUGH_QUOTA, ENOMEM) \ X(ERROR_NOT_FOUND, ENOENT) \ @@ -55,6 +56,7 @@ X(ERROR_NO_SYSTEM_RESOURCES, ENOMEM) \ X(ERROR_OPERATION_ABORTED, EINTR) \ X(ERROR_OUT_OF_PAPER, EACCES) \ + X(ERROR_PAGED_SYSTEM_RESOURCES, ENOMEM) \ X(ERROR_PAGEFILE_QUOTA, ENOMEM) \ X(ERROR_PATH_NOT_FOUND, ENOENT) \ X(ERROR_PIPE_NOT_CONNECTED, EPIPE) \ From d83871797c4daba8a5a2e55c9828338589b6ecfa Mon Sep 17 00:00:00 2001 From: Bert Belder Date: Mon, 4 Dec 2017 21:00:44 +0100 Subject: [PATCH 6/9] error: add return_handle_error(value, handle, [error]) macro This macro validates that `handle` is valid. If `handle` is valid, it behaves just like `return_error(value, [error])`. If `handle` is invalid, it sets errno to EBADF and GetLastError() returns ERROR_INVALID_HANDLE. --- src/error.c | 20 ++++++++++++++++++++ src/error.h | 11 ++++++++++- 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/src/error.c b/src/error.c index cd45cd5..d05db90 100644 --- a/src/error.c +++ b/src/error.c @@ -119,3 +119,23 @@ void err_set_win_error(DWORD error) { SetLastError(error); errno = err_map_win_error_to_errno(error); } + +int _check_handle(HANDLE handle) { + DWORD flags; + + /* GetHandleInformation() succeeds when passed INVALID_HANDLE_VALUE, so check + * for this condition explicitly. */ + if (handle == INVALID_HANDLE_VALUE) + return_error(-1, ERROR_INVALID_HANDLE); + + if (!GetHandleInformation(handle, &flags)) + return_error(-1); + + return 0; +} + +void err_validate_handle_and_set_win_error(HANDLE handle, DWORD error) { + if (_check_handle(handle) < 0) + return; + err_set_win_error(error); +} diff --git a/src/error.h b/src/error.h index 04ae9cb..9a4b3f0 100644 --- a/src/error.h +++ b/src/error.h @@ -11,10 +11,19 @@ err_set_win_error(error); \ return (value); \ } while (0) - #define return_error(value, ...) _return_error_helper(__VA_ARGS__ + 0, value) +#define _return_handle_error_helper(error, value, handle) \ + do { \ + err_validate_handle_and_set_win_error((HANDLE) handle, error); \ + return (value); \ + } while (0) +#define return_handle_error(value, handle, ...) \ + _return_handle_error_helper(__VA_ARGS__ + 0, value, handle) + WEPOLL_INTERNAL errno_t err_map_win_error_to_errno(DWORD error); WEPOLL_INTERNAL void err_set_win_error(DWORD error); +WEPOLL_INTERNAL void err_validate_handle_and_set_win_error(HANDLE handle, + DWORD error); #endif /* WEPOLL_ERROR_H_ */ From d5b5b605c0de1bd6d5d5c1fb96f85966f1d6440d Mon Sep 17 00:00:00 2001 From: Bert Belder Date: Mon, 4 Dec 2017 21:02:49 +0100 Subject: [PATCH 7/9] api: epoll_ctl_mod() report EBADF when modifying/deleting invalid socket --- src/port.c | 2 +- test/test-auto-drop-on-close.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/port.c b/src/port.c index 16fbea0..7e6bb06 100644 --- a/src/port.c +++ b/src/port.c @@ -348,7 +348,7 @@ ep_sock_t* ep_port_find_socket(ep_port_t* port_info, SOCKET socket) { ep_sock_t* sock_info = safe_container_of( tree_find(&port_info->sock_tree, socket), ep_sock_t, tree_node); if (sock_info == NULL) - return_error(NULL, ERROR_NOT_FOUND); + return_handle_error(NULL, socket, ERROR_NOT_FOUND); return sock_info; } diff --git a/test/test-auto-drop-on-close.c b/test/test-auto-drop-on-close.c index df2a782..2c38691 100644 --- a/test/test-auto-drop-on-close.c +++ b/test/test-auto-drop-on-close.c @@ -45,12 +45,12 @@ static void check_dropped(HANDLE ephnd, SOCKET sock) { /* Check that EPOLL_CTL_MOD fails. */ r = epoll_ctl(ephnd, EPOLL_CTL_MOD, sock, &ev); check(r < 0); - check(errno == ENOENT); /* TODO: should be EBADF. */ + check(errno == EBADF); /* Check that EPOLL_CTL_DEL fails. */ r = epoll_ctl(ephnd, EPOLL_CTL_DEL, sock, &ev); check(r < 0); - check(errno == ENOENT); /* TODO: should be EBADF. */ + check(errno == EBADF); } int main(void) { From 11d7a0395fa222360840d79c118921e8b9777861 Mon Sep 17 00:00:00 2001 From: Bert Belder Date: Mon, 4 Dec 2017 21:04:58 +0100 Subject: [PATCH 8/9] api: report EINVAL when ephnd is a valid handle but not an epoll instance --- src/api.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/api.c b/src/api.c index 7f16269..dce2fe5 100644 --- a/src/api.c +++ b/src/api.c @@ -66,7 +66,7 @@ int epoll_close(HANDLE ephnd) { tree_node = reflock_tree_del_and_ref(&_epoll_handle_tree, (uintptr_t) ephnd); if (tree_node == NULL) - return_error(-1, ERROR_INVALID_HANDLE); + return_handle_error(-1, ephnd, ERROR_INVALID_PARAMETER); port_info = _handle_tree_node_to_port(tree_node); ep_port_close(port_info); @@ -87,7 +87,7 @@ int epoll_ctl(HANDLE ephnd, int op, SOCKET sock, struct epoll_event* ev) { tree_node = reflock_tree_find_and_ref(&_epoll_handle_tree, (uintptr_t) ephnd); if (tree_node == NULL) - return_error(-1, ERROR_INVALID_HANDLE); + return_handle_error(-1, ephnd, ERROR_INVALID_PARAMETER); port_info = _handle_tree_node_to_port(tree_node); result = ep_port_ctl(port_info, op, sock, ev); @@ -111,7 +111,7 @@ int epoll_wait(HANDLE ephnd, tree_node = reflock_tree_find_and_ref(&_epoll_handle_tree, (uintptr_t) ephnd); if (tree_node == NULL) - return_error(-1, ERROR_INVALID_HANDLE); + return_handle_error(-1, ephnd, ERROR_INVALID_PARAMETER); port_info = _handle_tree_node_to_port(tree_node); result = ep_port_wait(port_info, events, maxevents, timeout); From bbeed7566cb0ed50fa5c18c0945c713971e8aeaa Mon Sep 17 00:00:00 2001 From: Bert Belder Date: Mon, 4 Dec 2017 21:07:43 +0100 Subject: [PATCH 9/9] test: add (incomplete) error code test --- test/test-error.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 test/test-error.c diff --git a/test/test-error.c b/test/test-error.c new file mode 100644 index 0000000..9096516 --- /dev/null +++ b/test/test-error.c @@ -0,0 +1,67 @@ +#include +#include +#include + +#include "test-util.h" +#include "util.h" +#include "wepoll.h" +#include "win.h" + +static void check_error(bool did_fail, + errno_t errno_value, + DWORD last_error_value) { + check(did_fail); + check(errno == errno_value); + check(GetLastError() == last_error_value); +} + +int main(void) { + const HANDLE bad_value = (HANDLE) 0xbadbad; + const HANDLE bad_type = CreateEventW(NULL, FALSE, FALSE, NULL); + check(bad_type != NULL && bad_type != INVALID_HANDLE_VALUE); + + { + /* Test epoll_create() errors. */ + HANDLE h; + h = epoll_create(-1); + check_error(h == NULL, EINVAL, ERROR_INVALID_PARAMETER); + h = epoll_create(0); + check_error(h == NULL, EINVAL, ERROR_INVALID_PARAMETER); + } + + { + /* Test epoll_create1() errors. */ + HANDLE h; + h = epoll_create1(-1); + check_error(h == NULL, EINVAL, ERROR_INVALID_PARAMETER); + h = epoll_create1(19); + check_error(h == NULL, EINVAL, ERROR_INVALID_PARAMETER); + } + + { + /* Test epoll_close() errors. */ + int r; + r = epoll_close(NULL); + check_error(r == -1, EBADF, ERROR_INVALID_HANDLE); + r = epoll_close(INVALID_HANDLE_VALUE); + check_error(r == -1, EBADF, ERROR_INVALID_HANDLE); + r = epoll_close(bad_value); + check_error(r == -1, EBADF, ERROR_INVALID_HANDLE); + r = epoll_close(bad_type); + check_error(r == -1, EINVAL, ERROR_INVALID_PARAMETER); + } + + { + /* Test epoll_ctl() errors. */ + /* TODO. */ + } + + { + /* Test epoll_wait() errors. */ + /* TODO. */ + } + + CloseHandle(bad_type); + + return 0; +}