[GitHub] [incubator-nuttx] anchao commented on a change in pull request #5434: net/tcp/udp: remove psock hook to avoid invalid reference

[GitBox <gi...@apache.org>]

anchao commented on a change in pull request #5434:
URL: https://github.com/apache/incubator-nuttx/pull/5434#discussion_r801229491



##########
File path: include/nuttx/net/net.h
##########
@@ -208,6 +208,10 @@ struct socket_conn_s
   FAR struct devif_callback_s *list_tail;
 
   /* Connection-specific content may follow */
+
+  /* Definitions of 8-bit socket flags */
+
+  uint8_t       s_flags;     /* See _SF_* definitions */

Review comment:
       done

##########
File path: net/icmp/icmp.h
##########
@@ -77,14 +78,11 @@ struct icmp_conn_s
 {
   /* Common prologue of all connection structures. */
 
-  dq_entry_t node;               /* Supports a doubly linked list */
-
   /* This is a list of ICMP callbacks.  Each callback represents a thread
    * that is stalled, waiting for a device-specific event.
    */

Review comment:
       done

##########
File path: net/local/local_accept.c
##########
@@ -181,8 +181,8 @@ int local_accept(FAR struct socket *psock, FAR struct sockaddr *addr,
                * block.
                */
 
-              ret = local_open_server_tx(conn,
-                                         _SS_ISNONBLOCK(psock->s_flags));
+              ret = local_open_server_tx(
+                      conn, _SS_ISNONBLOCK(conn->sconn.s_flags));

Review comment:
       warning by nxstyle check

##########
File path: net/bluetooth/bluetooth_sockif.c
##########
@@ -439,6 +439,7 @@ static int bluetooth_l2cap_bind(FAR struct socket *psock,
                                 FAR const struct sockaddr_l2 *iaddr,
                                 socklen_t addrlen)
 {
+  FAR struct socket_conn_s *sconn = psock->s_conn;

Review comment:
       > And let's add DEBUGASSERT(conn != NULL).
   
   done
   
   > should we change all sconn to conn to unity the term?
   
   I think sconn is better, “conn” is a common naming in most files

##########
File path: net/udp/udp_sendto_unbuffered.c
##########
@@ -260,17 +260,26 @@ ssize_t psock_udp_sendto(FAR struct socket *psock, FAR const void *buf,
                          size_t len, int flags,
                          FAR const struct sockaddr *to, socklen_t tolen)
 {
-  FAR struct udp_conn_s *conn;
+  FAR struct udp_conn_s *conn = psock->s_conn;
   struct sendto_s state;
   int ret;
 
+  /* Verify that the sockfd corresponds to valid, allocated socket */
+
+  if (psock == NULL || psock->s_type != SOCK_DGRAM ||

Review comment:
       sanity check copy from tcp

##########
File path: net/inet/inet_sockif.c
##########
@@ -718,7 +718,8 @@ static int inet_connect(FAR struct socket *psock,
         {
           /* Verify that the socket is not already connected */
 
-          if (_SS_ISCONNECTED(psock->s_flags))
+          if (_SS_ISCONNECTED(((FAR struct socket_conn_s *)
+                              psock->s_conn)->s_flags))

Review comment:
       done

##########
File path: net/tcp/tcp_monitor.c
##########
@@ -161,11 +159,14 @@ static uint16_t tcp_monitor_event(FAR struct net_driver_s *dev,
 
           /* Clear the socket error */
 
-          _SO_SETERRNO(psock, OK);

Review comment:
       ok, I will plan to improve the socket error handling on next patch, let us keep the current behavior first

##########
File path: net/udp/udp.h
##########
@@ -147,6 +147,10 @@ struct udp_conn_s
 
   sq_queue_t write_q;             /* Write buffering for UDP packets */
   FAR struct net_driver_s *dev;   /* Last device */
+
+  /* Callback instance for UDP sendto() */
+
+  FAR struct devif_callback_s *sndcb;

Review comment:
       yes, which guard around line 141-150
   
   https://github.com/apache/incubator-nuttx/pull/5434/files/dabe08a118f0d03b24d0bde0317bd2d83742eba9#diff-1597ffbc1b34e3e6e48bbcdaff385c423ab96b77e2c0ea7ba012c4c0a173094eL141-L150




-- 
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-unsubscribe@nuttx.apache.org

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