fjpanag commented on code in PR #7525:
URL: https://github.com/apache/nuttx/pull/7525#discussion_r1038983905


##########
net/tcp/tcp_conn.c:
##########
@@ -836,10 +863,25 @@ void tcp_free(FAR struct tcp_conn_s *conn)
     }
 #endif
 
-  /* Mark the connection available and put it into the free list */
+  /* Mark the connection available. */
 
   conn->tcpstateflags = TCP_CLOSED;
-  dq_addlast(&conn->sconn.node, &g_free_tcp_connections);
+
+  /* Check if this is a preallocated connection to store it in the free
+   * connections list, else deallocate it.
+   */
+
+#ifdef CONFIG_NET_ALLOC_CONNS
+  if ((conn->flags & TCP_PREALLOC) == 0)
+    {
+      kmm_free(conn);

Review Comment:
   > Yes, it's a problem. So, I think the better approach is move flag mark and 
memset to tcp_alloc_conn.
   
   I also thought of that, but now `memset()` is used for all connections (even 
those in the free list), every time (not only during the initial allocation).
   
   I am not sure whether code anywhere else works under the assumption that 
conns are `memset()` at this point, or it is just defensive coding.
   
   Also I am not sure whether
   ```g_tcp_connections[i].tcpstateflags = TCP_CLOSED;```
   has any meaning in line 605, or it will be overwritten by the memset (and 
thus it is dead code).



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to