Refactor add/remove conn/child code

Fix whitespace

pxyconn.c

@@ -284,29 +284,34 @@ pxy_conn_ctx_free_child(pxy_conn_child_ctx_t *ctx)
 	free(ctx);
 }
 
-static void NONNULL(1,2)
+static void NONNULL(1)
-pxy_conn_remove_child(pxy_conn_child_ctx_t *child, pxy_conn_child_ctx_t **head)
+pxy_conn_remove_child(pxy_conn_child_ctx_t *ctx)
 {
 #ifdef DEBUG_PROXY
-	log_dbg_level_printf(LOG_DBG_MODE_FINEST, "pxy_conn_remove_child: ENTER, child fd=%d, fd=%d\n", child->fd, child->conn->fd);
+	log_dbg_level_printf(LOG_DBG_MODE_FINEST, "pxy_conn_remove_child: ENTER, child fd=%d, fd=%d\n", ctx->fd, ctx->conn->fd);
 #endif /* DEBUG_PROXY */
 
-    if (child->fd == (*head)->fd) {
+	if (ctx->fd == ctx->conn->children->fd) {
-        *head = (*head)->next;
+		ctx->conn->children = ctx->conn->children->next;
-        return;
+		return;
-    }
+	}
-
+
-    pxy_conn_child_ctx_t *current = (*head)->next;
+	pxy_conn_child_ctx_t *current = ctx->conn->children->next;
-    pxy_conn_child_ctx_t *previous = *head;
+	pxy_conn_child_ctx_t *previous = ctx->conn->children;
-    while (current != NULL && previous != NULL) {
+	while (current != NULL && previous != NULL) {
-        if (child->fd == current->fd) {
+		if (ctx->fd == current->fd) {
-            previous->next = current->next;
+			previous->next = current->next;
-            return;
+			return;
-        }
+		}
-        previous = current;
+		previous = current;
-        current = current->next;
+		current = current->next;
-    }
+	}
-    return;
+	// This should never happen
+	log_err_level_printf(LOG_CRIT, "Cannot find child in conn children\n");
+#ifdef DEBUG_PROXY
+	log_dbg_level_printf(LOG_DBG_MODE_FINE, "pxy_conn_remove_child: Cannot find child in conn children, child fd=%d, fd=%d\n", ctx->fd, ctx->conn->fd);
+#endif /* DEBUG_PROXY */
+	assert(0);
 }
 
 static void
@@ -328,7 +333,7 @@ pxy_conn_free_child(pxy_conn_child_ctx_t *ctx)
 		ctx->src.bev = NULL;
 	}
 
-	pxy_conn_remove_child(ctx, &ctx->conn->children);
+	pxy_conn_remove_child(ctx);
 	pxy_conn_ctx_free_child(ctx);
 }
 
@@ -1060,7 +1065,7 @@ out:
  */
 static void
 pxy_listener_acceptcb_child(UNUSED struct evconnlistener *listener, evutil_socket_t fd,
-                        UNUSED struct sockaddr *peeraddr, UNUSED int peeraddrlen, void *arg)
+							UNUSED struct sockaddr *peeraddr, UNUSED int peeraddrlen, void *arg)
 {
 	pxy_conn_ctx_t *conn = arg;
 
@@ -1815,7 +1820,7 @@ ether_str(struct sockaddr_dl *sdl)
 	if (sdl->sdl_alen) {
 		cp = (u_char *)LLADDR(sdl);
 		snprintf(hbuf, sizeof(hbuf), "%02x:%02x:%02x:%02x:%02x:%02x",
-		    cp[0], cp[1], cp[2], cp[3], cp[4], cp[5]);
+			cp[0], cp[1], cp[2], cp[3], cp[4], cp[5]);
 		return strdup(hbuf);
 	} else {
 		return NULL;
@@ -2031,12 +2036,12 @@ pxy_conn_setup(evutil_socket_t fd,
 	/* prepare logging part 1 and user auth */
 	if (opts->pcaplog || opts->user_auth
 #ifndef WITHOUT_MIRROR
-	    || opts->mirrorif
+		|| opts->mirrorif
 #endif /* !WITHOUT_MIRROR */
 #ifdef HAVE_LOCAL_PROCINFO
-	    || opts->lprocinfo
+		|| opts->lprocinfo
 #endif /* HAVE_LOCAL_PROCINFO */
-	    ) {
+		) {
 		ctx->srcaddrlen = peeraddrlen;
 		memcpy(&ctx->srcaddr, peeraddr, ctx->srcaddrlen);
 	}

pxythrmgr.c

@@ -485,32 +485,51 @@ pxy_thrmgr_add_conn(pxy_conn_ctx_t *ctx)
 	pthread_mutex_unlock(&ctx->thr->mutex);
 }
 
-static void 
+static void NONNULL(1)
-pxy_thrmgr_remove_conn(pxy_conn_ctx_t *node, pxy_conn_ctx_t **head)
+pxy_thrmgr_remove_conn(pxy_conn_ctx_t *ctx)
 {
-	assert(node != NULL);
+	assert(ctx != NULL);
-	assert(*head != NULL);
+	assert(ctx->thr->conns != NULL);
+	assert(ctx->children == NULL);
 
+	if (ctx->in_thr_conns) {
 #ifdef DEBUG_PROXY
-	log_dbg_level_printf(LOG_DBG_MODE_FINEST, "pxy_thrmgr_remove_conn: Removing conn, id=%llu, fd=%d, child_fd=%d\n", node->id, node->fd, node->child_fd);
+		log_dbg_level_printf(LOG_DBG_MODE_FINEST, "pxy_thrmgr_remove_conn: Removing conn, id=%llu, fd=%d\n", ctx->id, ctx->fd);
 #endif /* DEBUG_PROXY */
-	
+
-	// @attention We may get multiple conns with the same fd combinations, so fds cannot uniquely define a conn; hence the need for unique ids.
+		// We increment thr load in pxy_thrmgr_add_conn() only (for parent conns)
-    if (node->id == (*head)->id) {
+		ctx->thr->load--;
-        *head = (*head)->next;
+		// Shouldn't need to reset the in_thr_conns flag, because the conn ctx will be freed next, but just in case
-        return;
+		ctx->in_thr_conns = 0;
-    }
+
-
+		// @attention We may get multiple conns with the same fd combinations, so fds cannot uniquely define a conn; hence the need for unique ids.
-    pxy_conn_ctx_t *current = (*head)->next;
+		if (ctx->id == ctx->thr->conns->id) {
-    pxy_conn_ctx_t *previous = *head;
+			ctx->thr->conns = ctx->thr->conns->next;
-    while (current != NULL && previous != NULL) {
+			return;
-        if (node->id == current->id) {
+		} else {
-            previous->next = current->next;
+			pxy_conn_ctx_t *current = ctx->thr->conns->next;
-            return;
+			pxy_conn_ctx_t *previous = ctx->thr->conns;
-        }
+			while (current != NULL && previous != NULL) {
-        previous = current;
+				if (ctx->id == current->id) {
-        current = current->next;
+					previous->next = current->next;
-    }
+					return;
+				}
+				previous = current;
+				current = current->next;
+			}
+			// This should never happen
+			log_err_level_printf(LOG_CRIT, "Cannot find conn in thr conns\n");
+#ifdef DEBUG_PROXY
+			log_dbg_level_printf(LOG_DBG_MODE_FINE, "pxy_thrmgr_remove_conn: Cannot find conn in thr conns, id=%llu, fd=%d\n", ctx->id, ctx->fd);
+#endif /* DEBUG_PROXY */
+			assert(0);
+		}
+	} else {
+		// This can happen if we are closing the conn after a fatal error before setting its event callback
+#ifdef DEBUG_PROXY
+		log_dbg_level_printf(LOG_DBG_MODE_FINEST, "pxy_thrmgr_remove_conn: Conn not in thr conns, id=%llu, fd=%d\n", ctx->id, ctx->fd);
+#endif /* DEBUG_PROXY */
+	}
 }
 
 /*
@@ -592,15 +611,7 @@ pxy_thrmgr_detach(pxy_conn_ctx_t *ctx)
 	log_dbg_level_printf(LOG_DBG_MODE_FINEST, "pxy_thrmgr_detach: ENTER\n");
 #endif /* DEBUG_PROXY */
 
-	assert(ctx->children == NULL);
+	pxy_thrmgr_remove_conn(ctx);
-
-	if (ctx->in_thr_conns) {
-		// We increment thr load in pxy_thrmgr_add_conn() only (for parent conns)
-		ctx->thr->load--;
-		pxy_thrmgr_remove_conn(ctx, &ctx->thr->conns);
-		// Shouldn't need to reset the in_thr_conns flag, because the conn ctx will be freed next, but just in case
-		ctx->in_thr_conns = 0;
-	}
 }
 
 void