From c4dc102a0b99b75d87cc93816bc6b8a90d5b8b03 Mon Sep 17 00:00:00 2001
From: Andy Green <andy@warmcat.com>
Date: Sun, 6 May 2018 07:19:21 +0800
Subject: [PATCH] windows: cleanup wrong and duplicated socket validity helpers
https://github.com/warmcat/libwebsockets/issues/1259
---
lib/core/libwebsockets.c | 7 +++---
lib/core/pollfd.c | 28 ++++++++++++++++--------
lib/core/private.h | 17 ++++++++------
lib/event-libs/libuv/libuv.c | 2 +-
lib/libwebsockets.h | 5 ++---
lib/plat/lws-plat-unix.c | 2 +-
lib/roles/cgi/cgi-server.c | 2 +-
lib/roles/http/client/client-handshake.c | 4 ++--
lib/roles/http/server/parsers.c | 5 +++--
lib/roles/http/server/server.c | 5 ++++-
lib/roles/listen/ops-listen.c | 3 ++-
lib/roles/ws/ops-ws.c | 2 +-
lib/tls/tls.c | 2 +-
13 files changed, 51 insertions(+), 33 deletions(-)
diff --git a/lib/core/libwebsockets.c b/lib/core/libwebsockets.c
index 4360e958..8eedbf82 100644
--- a/lib/core/libwebsockets.c
+++ b/lib/core/libwebsockets.c
@@ -751,7 +751,7 @@ just_kill_connection:
__func__, wsi, (int)(long)wsi->desc.sockfd,
lwsi_state(wsi));
if (!wsi->socket_is_permanently_unusable &&
- lws_sockfd_valid(wsi->desc.sockfd)) {
+ lws_socket_is_valid(wsi->desc.sockfd)) {
wsi->socket_is_permanently_unusable = 1;
n = shutdown(wsi->desc.sockfd, SHUT_WR);
}
@@ -767,7 +767,7 @@ just_kill_connection:
#if !defined(_WIN32_WCE) && !defined(LWS_WITH_ESP32)
/* libuv: no event available to guarantee completion */
if (!wsi->socket_is_permanently_unusable &&
- lws_sockfd_valid(wsi->desc.sockfd) &&
+ lws_socket_is_valid(wsi->desc.sockfd) &&
lwsi_state(wsi) != LRS_SHUTDOWN &&
context->event_loop_ops->periodic_events_available) {
__lws_change_pollfd(wsi, LWS_POLLOUT, LWS_POLLIN);
@@ -3326,7 +3326,8 @@ lws_stats_log_dump(struct lws_context *context)
context->updated = 1;
while (v) {
- if (v->lserv_wsi) {
+ if (v->lserv_wsi &&
+ v->lserv_wsi->position_in_fds_table != LWS_NO_FDS_POS) {
struct lws_context_per_thread *pt =
&context->pt[(int)v->lserv_wsi->tsi];
diff --git a/lib/core/pollfd.c b/lib/core/pollfd.c
index fbc6132a..2a632ce8 100644
--- a/lib/core/pollfd.c
+++ b/lib/core/pollfd.c
@@ -33,7 +33,13 @@ _lws_change_pollfd(struct lws *wsi, int _and, int _or, struct lws_pollargs *pa)
struct lws_pollfd *pfd;
int sampled_tid, tid;
- if (!wsi || wsi->position_in_fds_table < 0)
+ if (!wsi)
+ return 0;
+
+ assert(wsi->position_in_fds_table == LWS_NO_FDS_POS ||
+ wsi->position_in_fds_table >= 0);
+
+ if (wsi->position_in_fds_table == LWS_NO_FDS_POS)
return 0;
if (((volatile struct lws *)wsi)->handling_pollout &&
@@ -60,8 +66,7 @@ _lws_change_pollfd(struct lws *wsi, int _and, int _or, struct lws_pollargs *pa)
context = wsi->context;
pt = &context->pt[(int)wsi->tsi];
- assert(wsi->position_in_fds_table >= 0 &&
- wsi->position_in_fds_table < (int)pt->fds_count);
+ assert(wsi->position_in_fds_table < (int)pt->fds_count);
#if !defined(LWS_WITH_LIBUV) && \
!defined(LWS_WITH_LIBEV) && \
@@ -316,6 +321,10 @@ __remove_wsi_socket_from_fds(struct lws *wsi)
/* the guy who is to be deleted's slot index in pt->fds */
m = wsi->position_in_fds_table;
+ /* these are the only valid possibilities for position_in_fds_table */
+ assert(m == LWS_NO_FDS_POS || (m >= 0 &&
+ (unsigned int)m < pt->fds_count));
+
if (context->event_loop_ops->io)
context->event_loop_ops->io(wsi,
LWS_EV_STOP | LWS_EV_READ | LWS_EV_WRITE |
@@ -325,7 +334,7 @@ __remove_wsi_socket_from_fds(struct lws *wsi)
__func__, wsi, wsi->desc.sockfd, wsi->position_in_fds_table,
pt->fds_count, pt->fds[pt->fds_count].fd);
- if (m != LWS_SOCK_INVALID) {
+ if (m != LWS_NO_FDS_POS) {
/* have the last guy take up the now vacant slot */
pt->fds[m] = pt->fds[pt->fds_count - 1];
@@ -335,16 +344,17 @@ __remove_wsi_socket_from_fds(struct lws *wsi)
/* end guy's "position in fds table" is now the deletion guy's old one */
end_wsi = wsi_from_fd(context, v);
if (!end_wsi) {
- lwsl_err("no wsi found for fd %d at pos %d, pt->fds_count=%d\n",
- (int)pt->fds[m].fd, m, pt->fds_count);
+ lwsl_err("no wsi for fd %d at pos %d, pt->fds_count=%d\n",
+ (int)pt->fds[m].fd, m, pt->fds_count);
assert(0);
} else
end_wsi->position_in_fds_table = m;
/* deletion guy's lws_lookup entry needs nuking */
delete_from_fd(context, wsi->desc.sockfd);
+
/* removed wsi has no position any more */
- wsi->position_in_fds_table = -1;
+ wsi->position_in_fds_table = LWS_NO_FDS_POS;
}
/* remove also from external POLL support via protocol 0 */
@@ -376,7 +386,7 @@ __lws_change_pollfd(struct lws *wsi, int _and, int _or)
int ret = 0;
if (!wsi || (!wsi->protocol && !wsi->event_pipe) ||
- wsi->position_in_fds_table < 0)
+ wsi->position_in_fds_table == LWS_NO_FDS_POS)
return 0;
context = lws_get_context(wsi);
@@ -458,7 +468,7 @@ lws_callback_on_writable(struct lws *wsi)
wsi = lws_get_network_wsi(wsi);
}
- if (wsi->position_in_fds_table < 0) {
+ if (wsi->position_in_fds_table == LWS_NO_FDS_POS) {
lwsl_debug("%s: failed to find socket %d\n", __func__,
wsi->desc.sockfd);
return -1;
diff --git a/lib/core/private.h b/lib/core/private.h
index b31b6938..d6b494ac 100644
--- a/lib/core/private.h
+++ b/lib/core/private.h
@@ -92,8 +92,8 @@
#define compatible_close(fd) closesocket(fd)
#define lws_set_blocking_send(wsi) wsi->sock_send_blocking = 1
- #define lws_socket_is_valid(x) (!!x)
#define LWS_SOCK_INVALID (INVALID_SOCKET)
+
#include <winsock2.h>
#include <ws2tcpip.h>
#include <windows.h>
@@ -189,7 +189,6 @@
#define lws_set_blocking_send(wsi)
- #define lws_socket_is_valid(x) (x >= 0)
#define LWS_SOCK_INVALID (-1)
#endif /* not windows */
@@ -203,6 +202,9 @@
#define strerror(x) ""
#endif
+
+#define lws_socket_is_valid(x) (x != LWS_SOCK_INVALID)
+
#include "libwebsockets.h"
#include "tls/private.h"
@@ -1069,11 +1071,6 @@ struct lws {
struct lws_lws_tls tls;
#endif
-#ifdef LWS_LATENCY
- unsigned long action_start;
- unsigned long latency_start;
-#endif
-
lws_sock_file_fd_type desc; /* .filefd / .sockfd */
#if defined(LWS_WITH_STATS)
uint64_t active_writable_req_us;
@@ -1085,7 +1082,13 @@ struct lws {
lws_usec_t pending_timer; /* hrtimer fires */
time_t pending_timeout_set; /* second-resolution timeout start */
+#ifdef LWS_LATENCY
+ unsigned long action_start;
+ unsigned long latency_start;
+#endif
+
/* ints */
+#define LWS_NO_FDS_POS (-1)
int position_in_fds_table;
unsigned int trunc_alloc_len; /* size of malloc */
unsigned int trunc_offset; /* where we are in terms of spilling */
diff --git a/lib/event-libs/libuv/libuv.c b/lib/event-libs/libuv/libuv.c
index b5c8b85f..57947d4e 100644
--- a/lib/event-libs/libuv/libuv.c
+++ b/lib/event-libs/libuv/libuv.c
@@ -562,7 +562,7 @@ elops_destroy_context2_uv(struct lws_context *context)
static int
elops_wsi_logical_close_uv(struct lws *wsi)
{
- if (wsi->parent_carries_io || !lws_sockfd_valid(wsi->desc.sockfd))
+ if (wsi->parent_carries_io || !lws_socket_is_valid(wsi->desc.sockfd))
return 0;
if (wsi->listener || wsi->event_pipe) {
diff --git a/lib/libwebsockets.h b/lib/libwebsockets.h
index c4f8bcbe..cf4cb518 100644
--- a/lib/libwebsockets.h
+++ b/lib/libwebsockets.h
@@ -481,7 +481,7 @@ typedef int64_t lws_usec_t;
#if defined(_WIN32)
typedef SOCKET lws_sockfd_type;
typedef HANDLE lws_filefd_type;
-#define lws_sockfd_valid(sfd) (!!sfd)
+
struct lws_pollfd {
lws_sockfd_type fd; /**< file descriptor */
SHORT events; /**< which events to respond to */
@@ -497,7 +497,7 @@ struct lws_pollfd {
typedef int lws_sockfd_type;
typedef int lws_filefd_type;
-#define lws_sockfd_valid(sfd) (sfd >= 0)
+
struct pollfd {
lws_sockfd_type fd; /**< fd related to */
short events; /**< which POLL... events to respond to */
@@ -701,7 +701,6 @@ extern void lws_esp32_leds_timer_cb(TimerHandle_t th);
#else
typedef int lws_sockfd_type;
typedef int lws_filefd_type;
-#define lws_sockfd_valid(sfd) (sfd >= 0)
#endif
#define lws_pollfd pollfd
diff --git a/lib/plat/lws-plat-unix.c b/lib/plat/lws-plat-unix.c
index 5b59f20a..bacc6af6 100644
--- a/lib/plat/lws-plat-unix.c
+++ b/lib/plat/lws-plat-unix.c
@@ -239,7 +239,7 @@ _lws_plat_service_tsi(struct lws_context *context, int timeout_ms, int tsi)
next = ftp->next;
pfd = &vpt->fds[ftp->fd_index];
- if (lws_sockfd_valid(pfd->fd)) {
+ if (lws_socket_is_valid(pfd->fd)) {
wsi = wsi_from_fd(context, pfd->fd);
if (wsi)
__lws_change_pollfd(wsi, ftp->_and, ftp->_or);
diff --git a/lib/roles/cgi/cgi-server.c b/lib/roles/cgi/cgi-server.c
index e5a466a7..964a3df8 100644
--- a/lib/roles/cgi/cgi-server.c
+++ b/lib/roles/cgi/cgi-server.c
@@ -91,7 +91,7 @@ lws_create_basic_wsi(struct lws_context *context, int tsi)
lws_role_transition(new_wsi, 0, LRS_ESTABLISHED, &role_ops_cgi);
new_wsi->hdr_parsing_completed = 0;
- new_wsi->position_in_fds_table = -1;
+ new_wsi->position_in_fds_table = LWS_NO_FDS_POS;
/*
* these can only be set once the protocol is known
diff --git a/lib/roles/http/client/client-handshake.c b/lib/roles/http/client/client-handshake.c
index 337ccea9..4830fc9e 100644
--- a/lib/roles/http/client/client-handshake.c
+++ b/lib/roles/http/client/client-handshake.c
@@ -577,7 +577,7 @@ oom4:
wsi->already_did_cce = 1;
}
/* take care that we might be inserted in fds already */
- if (wsi->position_in_fds_table != -1)
+ if (wsi->position_in_fds_table != LWS_NO_FDS_POS)
goto failed1;
lws_remove_from_timeout_list(wsi);
#if defined(LWS_ROLE_H1) || defined(LWS_ROLE_H2)
@@ -923,7 +923,7 @@ lws_client_connect_via_info(struct lws_client_connect_info *i)
wsi->user_space = NULL;
wsi->pending_timeout = NO_PENDING_TIMEOUT;
- wsi->position_in_fds_table = -1;
+ wsi->position_in_fds_table = LWS_NO_FDS_POS;
wsi->c_port = i->port;
wsi->vhost = i->vhost;
if (!wsi->vhost)
diff --git a/lib/roles/http/server/parsers.c b/lib/roles/http/server/parsers.c
index 054a839b..cb022e36 100644
--- a/lib/roles/http/server/parsers.c
+++ b/lib/roles/http/server/parsers.c
@@ -110,7 +110,8 @@ __lws_header_table_reset(struct lws *wsi, int autoservice)
time(&ah->assigned);
- if (lws_buflist_next_segment_len(&wsi->buflist, NULL) &&
+ if (wsi->position_in_fds_table != LWS_NO_FDS_POS &&
+ lws_buflist_next_segment_len(&wsi->buflist, NULL) &&
autoservice) {
lwsl_debug("%s: service on readbuf ah\n", __func__);
@@ -366,7 +367,7 @@ int __lws_header_table_detach(struct lws *wsi, int autoservice)
#endif
/* clients acquire the ah and then insert themselves in fds table... */
- if (wsi->position_in_fds_table != -1) {
+ if (wsi->position_in_fds_table != LWS_NO_FDS_POS) {
lwsl_info("%s: Enabling %p POLLIN\n", __func__, wsi);
/* he has been stuck waiting for an ah, but now his wait is
diff --git a/lib/roles/http/server/server.c b/lib/roles/http/server/server.c
index f55a1371..350af3cd 100644
--- a/lib/roles/http/server/server.c
+++ b/lib/roles/http/server/server.c
@@ -1695,7 +1695,7 @@ lws_create_new_server_wsi(struct lws_vhost *vhost, int fixed_tsi)
new_wsi->protocol = vhost->protocols;
new_wsi->user_space = NULL;
new_wsi->desc.sockfd = LWS_SOCK_INVALID;
- new_wsi->position_in_fds_table = -1;
+ new_wsi->position_in_fds_table = LWS_NO_FDS_POS;
vhost->context->count_wsi_allocated++;
@@ -2075,6 +2075,9 @@ adopt_socket_readbuf(struct lws *wsi, const char *readbuf, size_t len)
if (!readbuf || len == 0)
return wsi;
+ if (wsi->position_in_fds_table == LWS_NO_FDS_POS)
+ return wsi;
+
pt = &wsi->context->pt[(int)wsi->tsi];
n = lws_buflist_append_segment(&wsi->buflist, (const uint8_t *)readbuf, len);
diff --git a/lib/roles/listen/ops-listen.c b/lib/roles/listen/ops-listen.c
index 0e463231..dbeb9753 100644
--- a/lib/roles/listen/ops-listen.c
+++ b/lib/roles/listen/ops-listen.c
@@ -124,7 +124,7 @@ rops_handle_POLLIN_listen(struct lws_context_per_thread *pt, struct lws *wsi,
fd.sockfd = accept_fd;
cwsi = lws_adopt_descriptor_vhost(wsi->vhost, opts, fd,
- NULL, NULL);
+ NULL, NULL);
if (!cwsi)
/* already closed cleanly as necessary */
return LWS_HPI_RET_WSI_ALREADY_DIED;
@@ -139,6 +139,7 @@ rops_handle_POLLIN_listen(struct lws_context_per_thread *pt, struct lws *wsi,
__func__, cwsi, cwsi->wsistate, cwsi->role_ops->name);
} while (pt->fds_count < context->fd_limit_per_thread - 1 &&
+ wsi->position_in_fds_table != LWS_NO_FDS_POS &&
lws_poll_listen_fd(&pt->fds[wsi->position_in_fds_table]) > 0);
return LWS_HPI_RET_HANDLED;
diff --git a/lib/roles/ws/ops-ws.c b/lib/roles/ws/ops-ws.c
index fd036c31..310a3bea 100644
--- a/lib/roles/ws/ops-ws.c
+++ b/lib/roles/ws/ops-ws.c
@@ -1448,7 +1448,7 @@ rops_service_flag_pending_ws(struct lws_context *context, int tsi)
* not flowcontrolled, fake their POLLIN status
*/
wsi = pt->ws.rx_draining_ext_list;
- while (wsi) {
+ while (wsi && wsi->position_in_fds_table != LWS_NO_FDS_POS) {
pt->fds[wsi->position_in_fds_table].revents |=
pt->fds[wsi->position_in_fds_table].events & LWS_POLLIN;
if (pt->fds[wsi->position_in_fds_table].revents & LWS_POLLIN)
diff --git a/lib/tls/tls.c b/lib/tls/tls.c
index c865c89d..640a2839 100644
--- a/lib/tls/tls.c
+++ b/lib/tls/tls.c
@@ -34,7 +34,7 @@ lws_tls_fake_POLLIN_for_buffered(struct lws_context_per_thread *pt)
int ret = 0;
wsi = pt->tls.pending_read_list;
- while (wsi) {
+ while (wsi && wsi->position_in_fds_table != LWS_NO_FDS_POS) {
wsi_next = wsi->tls.pending_read_list_next;
pt->fds[wsi->position_in_fds_table].revents |=
pt->fds[wsi->position_in_fds_table].events & LWS_POLLIN;
--
GitLab