windows: cleanup wrong and duplicated socket validity helpers

https://github.com/warmcat/libwebsockets/issues/1259

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];

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;

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 */

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) {

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

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);

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

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)

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

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);

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;

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)

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;