From d448bdf8d895b7e889f91b7c41f471a4b340e466 Mon Sep 17 00:00:00 2001 From: Bert Belder Date: Thu, 7 Dec 2017 21:37:58 +0100 Subject: [PATCH 1/7] error: make err_check_handle() an internal api --- src/error.c | 4 ++-- src/error.h | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/error.c b/src/error.c index 40b0ea2..b0cae2f 100644 --- a/src/error.c +++ b/src/error.c @@ -121,7 +121,7 @@ void err_set_win_error(DWORD error) { errno = err_map_win_error_to_errno(error); } -int _check_handle(HANDLE handle) { +int err_check_handle(HANDLE handle) { DWORD flags; /* GetHandleInformation() succeeds when passed INVALID_HANDLE_VALUE, so check @@ -136,7 +136,7 @@ int _check_handle(HANDLE handle) { } void err_validate_handle_and_set_win_error(HANDLE handle, DWORD error) { - if (_check_handle(handle) < 0) + if (err_check_handle(handle) < 0) return; err_set_win_error(error); } diff --git a/src/error.h b/src/error.h index 9a4b3f0..c5ca688 100644 --- a/src/error.h +++ b/src/error.h @@ -23,6 +23,7 @@ WEPOLL_INTERNAL errno_t err_map_win_error_to_errno(DWORD error); WEPOLL_INTERNAL void err_set_win_error(DWORD error); +WEPOLL_INTERNAL int err_check_handle(HANDLE handle); WEPOLL_INTERNAL void err_validate_handle_and_set_win_error(HANDLE handle, DWORD error); From 157cadf4e425c49772bd22c57bdd201b5e2c8219 Mon Sep 17 00:00:00 2001 From: Bert Belder Date: Thu, 7 Dec 2017 21:55:09 +0100 Subject: [PATCH 2/7] api: always check handle validity when epoll_ctl_mod() fails --- src/afd.c | 2 +- src/api.c | 24 ++++++++++++++++++------ src/port.c | 4 ++-- test/test-error.c | 3 ++- 4 files changed, 23 insertions(+), 10 deletions(-) diff --git a/src/afd.c b/src/afd.c index a2f9d24..425ec1c 100644 --- a/src/afd.c +++ b/src/afd.c @@ -116,7 +116,7 @@ static ssize_t _afd_get_protocol_info(SOCKET socket, SO_PROTOCOL_INFOW, (char*) protocol_info, &opt_len) != 0) - return_handle_error(-1, socket); + return_error(-1); id = -1; for (size_t i = 0; i < array_count(AFD_PROVIDER_GUID_LIST); i++) { diff --git a/src/api.c b/src/api.c index 10438de..25e8d20 100644 --- a/src/api.c +++ b/src/api.c @@ -79,22 +79,34 @@ int epoll_close(HANDLE ephnd) { int epoll_ctl(HANDLE ephnd, int op, SOCKET sock, struct epoll_event* ev) { reflock_tree_node_t* tree_node; ep_port_t* port_info; - int result; + int r; if (init() < 0) return -1; tree_node = reflock_tree_find_and_ref(&_epoll_handle_tree, (uintptr_t) ephnd); - if (tree_node == NULL) - return_handle_error(-1, ephnd, ERROR_INVALID_PARAMETER); - port_info = _handle_tree_node_to_port(tree_node); + if (tree_node == NULL) { + err_set_win_error(ERROR_INVALID_PARAMETER); + goto err; + } - result = ep_port_ctl(port_info, op, sock, ev); + port_info = _handle_tree_node_to_port(tree_node); + r = ep_port_ctl(port_info, op, sock, ev); reflock_tree_node_unref(tree_node); - return result; + if (r < 0) + goto err; + + return 0; + +err: + /* On Linux, in the case of epoll_ctl_mod(), EBADF takes precendence over + * other errors. Wepoll copies this behavior. */ + err_check_handle(ephnd); + err_check_handle((HANDLE) sock); + return -1; } int epoll_wait(HANDLE ephnd, diff --git a/src/port.c b/src/port.c index f0631b7..4505321 100644 --- a/src/port.c +++ b/src/port.c @@ -320,7 +320,7 @@ static int _ep_port_ctl_op(ep_port_t* port_info, case EPOLL_CTL_DEL: return _ep_port_ctl_del(port_info, sock); default: - return_handle_error(-1, sock, ERROR_INVALID_PARAMETER); + return_error(-1, ERROR_INVALID_PARAMETER); } } @@ -355,7 +355,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_handle_error(NULL, socket, ERROR_NOT_FOUND); + return_error(NULL, ERROR_NOT_FOUND); return sock_info; } diff --git a/test/test-error.c b/test/test-error.c index b15cff9..f2f95c9 100644 --- a/test/test-error.c +++ b/test/test-error.c @@ -101,7 +101,8 @@ int main(void) { check_error(r < 0, EBADF, ERROR_INVALID_HANDLE); r = epoll_ctl(valid_ephnd, -1, INVALID_SOCKET, &ev); check_error(r < 0, EBADF, ERROR_INVALID_HANDLE); - /* TODO: bad `ephnd` type with invalid `sock`. */ + r = epoll_ctl(bad_type, -1, INVALID_SOCKET, &ev); + check_error(r < 0, EBADF, ERROR_INVALID_HANDLE); r = closesocket(sock_valid); check(r == 0); From 093e6f52405236fab04acfa69468657ffe914b24 Mon Sep 17 00:00:00 2001 From: Bert Belder Date: Thu, 7 Dec 2017 22:03:45 +0100 Subject: [PATCH 3/7] api: explicitly check handle validity in epoll_close() --- src/api.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/api.c b/src/api.c index 25e8d20..378eb6f 100644 --- a/src/api.c +++ b/src/api.c @@ -65,15 +65,21 @@ int epoll_close(HANDLE ephnd) { return -1; tree_node = reflock_tree_del_and_ref(&_epoll_handle_tree, (uintptr_t) ephnd); - if (tree_node == NULL) - return_handle_error(-1, ephnd, ERROR_INVALID_PARAMETER); - port_info = _handle_tree_node_to_port(tree_node); + if (tree_node == NULL) { + err_set_win_error(ERROR_INVALID_PARAMETER); + goto err; + } + port_info = _handle_tree_node_to_port(tree_node); ep_port_close(port_info); reflock_tree_node_unref_and_destroy(tree_node); return ep_port_delete(port_info); + +err: + err_check_handle(ephnd); + return -1; } int epoll_ctl(HANDLE ephnd, int op, SOCKET sock, struct epoll_event* ev) { From 0f68b70114b683729a750d342af39d5cd7720e9b Mon Sep 17 00:00:00 2001 From: Bert Belder Date: Thu, 7 Dec 2017 22:04:17 +0100 Subject: [PATCH 4/7] api: explicitly check handle validity in epoll_wait() --- src/api.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/api.c b/src/api.c index 378eb6f..ffdd96b 100644 --- a/src/api.c +++ b/src/api.c @@ -121,7 +121,7 @@ int epoll_wait(HANDLE ephnd, int timeout) { reflock_tree_node_t* tree_node; ep_port_t* port_info; - int result; + int num_events; if (maxevents <= 0) return_error(-1, ERROR_INVALID_PARAMETER); @@ -131,13 +131,22 @@ int epoll_wait(HANDLE ephnd, tree_node = reflock_tree_find_and_ref(&_epoll_handle_tree, (uintptr_t) ephnd); - if (tree_node == NULL) - return_handle_error(-1, ephnd, ERROR_INVALID_PARAMETER); - port_info = _handle_tree_node_to_port(tree_node); + if (tree_node == NULL) { + err_set_win_error(ERROR_INVALID_PARAMETER); + goto err; + } - result = ep_port_wait(port_info, events, maxevents, timeout); + port_info = _handle_tree_node_to_port(tree_node); + num_events = ep_port_wait(port_info, events, maxevents, timeout); reflock_tree_node_unref(tree_node); - return result; + if (num_events < 0) + goto err; + + return num_events; + +err: + err_check_handle(ephnd); + return -1; } From 7dcc3d5eab873d942879aedda6139128230756bb Mon Sep 17 00:00:00 2001 From: Bert Belder Date: Thu, 7 Dec 2017 22:06:11 +0100 Subject: [PATCH 5/7] error: remove return_handle_error() and helpers --- src/error.c | 6 ------ src/error.h | 11 +---------- 2 files changed, 1 insertion(+), 16 deletions(-) diff --git a/src/error.c b/src/error.c index b0cae2f..6b81179 100644 --- a/src/error.c +++ b/src/error.c @@ -134,9 +134,3 @@ int err_check_handle(HANDLE handle) { return 0; } - -void err_validate_handle_and_set_win_error(HANDLE handle, DWORD error) { - if (err_check_handle(handle) < 0) - return; - err_set_win_error(error); -} diff --git a/src/error.h b/src/error.h index c5ca688..1e4591b 100644 --- a/src/error.h +++ b/src/error.h @@ -11,20 +11,11 @@ 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) +#define return_error(value, ...) _return_error_helper(__VA_ARGS__ + 0, value) WEPOLL_INTERNAL errno_t err_map_win_error_to_errno(DWORD error); WEPOLL_INTERNAL void err_set_win_error(DWORD error); WEPOLL_INTERNAL int err_check_handle(HANDLE handle); -WEPOLL_INTERNAL void err_validate_handle_and_set_win_error(HANDLE handle, - DWORD error); #endif /* WEPOLL_ERROR_H_ */ From 937b9a27fa6adea74bebd745f3acf3a151a0bf0d Mon Sep 17 00:00:00 2001 From: Bert Belder Date: Thu, 7 Dec 2017 22:26:08 +0100 Subject: [PATCH 6/7] test: improve test-error readability --- test/test-error.c | 111 +++++++++++++++++++++++++--------------------- 1 file changed, 61 insertions(+), 50 deletions(-) diff --git a/test/test-error.c b/test/test-error.c index f2f95c9..77be3df 100644 --- a/test/test-error.c +++ b/test/test-error.c @@ -15,9 +15,22 @@ static void check_error(bool did_fail, } 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); + HANDLE event_handle; + HANDLE ep_null, ep_hinv, ep_badv, ep_type; + SOCKET sock_null, sock_hinv, sock_badv, sock_type; + + event_handle = CreateEventW(NULL, FALSE, FALSE, NULL); + check(event_handle != NULL && event_handle != INVALID_HANDLE_VALUE); + + ep_null = NULL; + ep_hinv = INVALID_HANDLE_VALUE; + ep_badv = (HANDLE) 0xbadbad; + ep_type = event_handle; + + sock_null = (SOCKET) NULL; + sock_hinv = INVALID_SOCKET; + sock_badv = (SOCKET) 0xbadbad; + sock_type = (SOCKET) event_handle; { /* Test epoll_create() errors. */ @@ -40,124 +53,122 @@ int main(void) { { /* Test epoll_close() errors. */ int r; - r = epoll_close(NULL); + r = epoll_close(ep_null); check_error(r == -1, EBADF, ERROR_INVALID_HANDLE); - r = epoll_close(INVALID_HANDLE_VALUE); + r = epoll_close(ep_hinv); check_error(r == -1, EBADF, ERROR_INVALID_HANDLE); - r = epoll_close(bad_value); + r = epoll_close(ep_badv); check_error(r == -1, EBADF, ERROR_INVALID_HANDLE); - r = epoll_close(bad_type); + r = epoll_close(ep_type); check_error(r == -1, EINVAL, ERROR_INVALID_PARAMETER); } { /* Test epoll_ctl() errors. */ - HANDLE valid_ephnd; - SOCKET sock_valid; - SOCKET sock_bad = (SOCKET) 0xbadbad; - SOCKET sock_nonsock = (SOCKET) bad_type; + HANDLE ep_good; + SOCKET sock_good; struct epoll_event ev; int r; - valid_ephnd = epoll_create1(0); - check(valid_ephnd != NULL); + ep_good = epoll_create1(0); + check(ep_good != ep_null); - sock_valid = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP); - check(sock_valid != INVALID_SOCKET); + sock_good = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP); + check(sock_good != sock_hinv); ev.data.u64 = 0; ev.events = 0; /* Invalid `ephnd` */ - r = epoll_ctl(NULL, EPOLL_CTL_ADD, sock_valid, &ev); + r = epoll_ctl(ep_null, EPOLL_CTL_ADD, sock_good, &ev); check_error(r < 0, EBADF, ERROR_INVALID_HANDLE); - r = epoll_ctl(INVALID_HANDLE_VALUE, EPOLL_CTL_ADD, sock_valid, &ev); + r = epoll_ctl(ep_hinv, EPOLL_CTL_ADD, sock_good, &ev); check_error(r < 0, EBADF, ERROR_INVALID_HANDLE); - r = epoll_ctl(bad_value, EPOLL_CTL_ADD, sock_valid, &ev); + r = epoll_ctl(ep_badv, EPOLL_CTL_ADD, sock_good, &ev); check_error(r < 0, EBADF, ERROR_INVALID_HANDLE); - r = epoll_ctl(bad_type, EPOLL_CTL_ADD, sock_valid, &ev); + r = epoll_ctl(ep_type, EPOLL_CTL_ADD, sock_good, &ev); check_error(r < 0, EINVAL, ERROR_INVALID_PARAMETER); /* Invalid `sock` */ - r = epoll_ctl(valid_ephnd, EPOLL_CTL_ADD, 0, &ev); + r = epoll_ctl(ep_good, EPOLL_CTL_ADD, sock_null, &ev); check_error(r < 0, EBADF, ERROR_INVALID_HANDLE); - r = epoll_ctl(valid_ephnd, EPOLL_CTL_ADD, INVALID_SOCKET, &ev); + r = epoll_ctl(ep_good, EPOLL_CTL_ADD, sock_hinv, &ev); check_error(r < 0, EBADF, ERROR_INVALID_HANDLE); - r = epoll_ctl(valid_ephnd, EPOLL_CTL_ADD, sock_bad, &ev); + r = epoll_ctl(ep_good, EPOLL_CTL_ADD, sock_badv, &ev); check_error(r < 0, EBADF, ERROR_INVALID_HANDLE); - r = epoll_ctl(valid_ephnd, EPOLL_CTL_ADD, sock_nonsock, &ev); + r = epoll_ctl(ep_good, EPOLL_CTL_ADD, sock_type, &ev); check_error(r < 0, ENOTSOCK, WSAENOTSOCK); /* Invalid `op` */ - r = epoll_ctl(valid_ephnd, -1, sock_valid, &ev); + r = epoll_ctl(ep_good, -1, sock_good, &ev); check_error(r < 0, EINVAL, ERROR_INVALID_PARAMETER); - r = epoll_ctl(valid_ephnd, 0, sock_valid, &ev); + r = epoll_ctl(ep_good, 0, sock_good, &ev); check_error(r < 0, EINVAL, ERROR_INVALID_PARAMETER); - r = epoll_ctl(valid_ephnd, 4, sock_valid, &ev); + r = epoll_ctl(ep_good, 4, sock_good, &ev); check_error(r < 0, EINVAL, ERROR_INVALID_PARAMETER); /* Precedence - `EBADF` before `EINVAL`. */ - r = epoll_ctl(INVALID_HANDLE_VALUE, -1, sock_valid, &ev); + r = epoll_ctl(ep_hinv, -1, sock_good, &ev); check_error(r < 0, EBADF, ERROR_INVALID_HANDLE); - r = epoll_ctl(valid_ephnd, -1, INVALID_SOCKET, &ev); + r = epoll_ctl(ep_good, -1, sock_hinv, &ev); check_error(r < 0, EBADF, ERROR_INVALID_HANDLE); - r = epoll_ctl(bad_type, -1, INVALID_SOCKET, &ev); + r = epoll_ctl(ep_type, -1, sock_hinv, &ev); check_error(r < 0, EBADF, ERROR_INVALID_HANDLE); - r = closesocket(sock_valid); + r = closesocket(sock_good); check(r == 0); - r = epoll_close(valid_ephnd); + r = epoll_close(ep_good); check(r == 0); } { /* Test epoll_wait() errors. */ - HANDLE valid_ephnd; + HANDLE ep_good; struct epoll_event evs[1]; int r; - valid_ephnd = epoll_create1(0); - check(valid_ephnd != NULL); + ep_good = epoll_create1(0); + check(ep_good != ep_null); /* Invalid `ephnd` */ - r = epoll_wait(NULL, evs, 1, 0); + r = epoll_wait(ep_null, evs, 1, 0); check_error(r == -1, EBADF, ERROR_INVALID_HANDLE); - r = epoll_wait(INVALID_HANDLE_VALUE, evs, 1, 0); + r = epoll_wait(ep_hinv, evs, 1, 0); check_error(r == -1, EBADF, ERROR_INVALID_HANDLE); - r = epoll_wait(bad_value, evs, 1, 0); + r = epoll_wait(ep_badv, evs, 1, 0); check_error(r == -1, EBADF, ERROR_INVALID_HANDLE); - r = epoll_wait(bad_type, evs, 1, 0); + r = epoll_wait(ep_type, evs, 1, 0); check_error(r == -1, EINVAL, ERROR_INVALID_PARAMETER); /* Zero `maxevents` */ - r = epoll_wait(valid_ephnd, evs, 0, 0); + r = epoll_wait(ep_good, evs, 0, 0); check_error(r == -1, EINVAL, ERROR_INVALID_PARAMETER); - r = epoll_wait(NULL, evs, 0, 0); + r = epoll_wait(ep_null, evs, 0, 0); check_error(r == -1, EINVAL, ERROR_INVALID_PARAMETER); - r = epoll_wait(INVALID_HANDLE_VALUE, evs, 0, 0); + r = epoll_wait(ep_hinv, evs, 0, 0); check_error(r == -1, EINVAL, ERROR_INVALID_PARAMETER); - r = epoll_wait(bad_value, evs, 0, 0); + r = epoll_wait(ep_badv, evs, 0, 0); check_error(r == -1, EINVAL, ERROR_INVALID_PARAMETER); - r = epoll_wait(bad_type, evs, 0, 0); + r = epoll_wait(ep_type, evs, 0, 0); check_error(r == -1, EINVAL, ERROR_INVALID_PARAMETER); /* Negative `maxevents` */ - r = epoll_wait(valid_ephnd, evs, -1, 0); + r = epoll_wait(ep_good, evs, -1, 0); check_error(r == -1, EINVAL, ERROR_INVALID_PARAMETER); - r = epoll_wait(NULL, evs, -1, 0); + r = epoll_wait(ep_null, evs, -1, 0); check_error(r == -1, EINVAL, ERROR_INVALID_PARAMETER); - r = epoll_wait(INVALID_HANDLE_VALUE, evs, -1, 0); + r = epoll_wait(ep_hinv, evs, -1, 0); check_error(r == -1, EINVAL, ERROR_INVALID_PARAMETER); - r = epoll_wait(bad_value, evs, -1, 0); + r = epoll_wait(ep_badv, evs, -1, 0); check_error(r == -1, EINVAL, ERROR_INVALID_PARAMETER); - r = epoll_wait(bad_type, evs, -1, 0); + r = epoll_wait(ep_type, evs, -1, 0); check_error(r == -1, EINVAL, ERROR_INVALID_PARAMETER); - r = epoll_close(valid_ephnd); + r = epoll_close(ep_good); check(r == 0); } - CloseHandle(bad_type); + CloseHandle(event_handle); return 0; } From 658775faee40e5a9f1b499dd5bc1a66e53066730 Mon Sep 17 00:00:00 2001 From: Bert Belder Date: Thu, 7 Dec 2017 22:30:54 +0100 Subject: [PATCH 7/7] test: add epoll_ctl_mod() EEXIST and ENOENT tests to test-error --- test/test-error.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/test/test-error.c b/test/test-error.c index 77be3df..dc6b852 100644 --- a/test/test-error.c +++ b/test/test-error.c @@ -115,6 +115,20 @@ int main(void) { r = epoll_ctl(ep_type, -1, sock_hinv, &ev); check_error(r < 0, EBADF, ERROR_INVALID_HANDLE); + /* Socket already in epoll set. */ + r = epoll_ctl(ep_good, EPOLL_CTL_ADD, sock_good, &ev); + check(r == 0); + r = epoll_ctl(ep_good, EPOLL_CTL_ADD, sock_good, &ev); + check_error(r < 0, EEXIST, ERROR_ALREADY_EXISTS); + + /* Socket not in epoll set. */ + r = epoll_ctl(ep_good, EPOLL_CTL_DEL, sock_good, NULL); + check(r == 0); + r = epoll_ctl(ep_good, EPOLL_CTL_MOD, sock_good, &ev); + check_error(r < 0, ENOENT, ERROR_NOT_FOUND); + r = epoll_ctl(ep_good, EPOLL_CTL_DEL, sock_good, NULL); + check_error(r < 0, ENOENT, ERROR_NOT_FOUND); + r = closesocket(sock_good); check(r == 0); r = epoll_close(ep_good);