casaroli commented on code in PR #21043:
URL: https://github.com/apache/nuttx/pull/21043#discussion_r4392212657


##########
net/devif/devif_poll.c:
##########
@@ -316,7 +317,27 @@ devif_poll_can_connections(FAR struct net_driver_s *dev,
 
           if (dev->d_len > 0)
             {
-              bstop = callback(dev);
+              if (!IFF_IS_LOOPBACK(dev->d_flags)
+#ifdef CONFIG_NET_CANPROTO_OPTIONS
+                  && _SO_GETOPT(can_conn->sconn.s_options, CAN_RAW_LOOPBACK)
+#endif
+                 )
+                {
+                  FAR struct iob_s *iob = can_iob_clone(dev);
+
+                  bstop = callback(dev);
+
+                  if (bstop && iob != NULL)

Review Comment:
   The clone is used and freed only when `callback()` returns non-zero; 
`fdcan_txpoll()` (stm32h7) returns 0 after a send, so each frame leaks one IOB.
   
   Fix: loop back based on whether the driver took the frame, and 
`iob_free_chain()` the clone on every other path.



##########
net/can/can_sockif.c:
##########
@@ -346,6 +356,65 @@ static int can_bind(FAR struct socket *psock,
   return conn->dev == NULL && canaddr->can_ifindex != 0 ? -ENODEV : OK;
 }
 
+/****************************************************************************
+ * Name: can_getsockname
+ *
+ * Description:
+ *   The can_getsockname() function retrieves the locally-bound name of the
+ *   specified CAN socket, stores this address in the sockaddr structure
+ *   pointed to by the 'addr' argument, and stores the length of this
+ *   address in the object pointed to by the 'addrlen' argument.
+ *
+ * Input Parameters:
+ *   psock    Socket structure of the socket to be queried
+ *   addr     sockaddr structure to receive data [out]
+ *   addrlen  Length of sockaddr structure [in/out]
+ *
+ * Returned Value:
+ *   On success, 0 is returned.  Otherwise, a negated errno value is
+ *   returned.
+ *
+ ****************************************************************************/
+
+static int can_getsockname(FAR struct socket *psock,
+                           FAR struct sockaddr *addr,
+                           FAR socklen_t *addrlen)
+{
+  FAR struct sockaddr_can *canaddr;
+  FAR struct can_conn_s *conn;
+
+  DEBUGASSERT(*addrlen >= sizeof(struct sockaddr_can));

Review Comment:
   `*addrlen` comes from the user and the DEBUGASSERT is not in release builds, 
so the `memset()` overruns a short buffer.
   
   Fix: return `-EINVAL` when `*addrlen < sizeof(struct sockaddr_can)`, as 
`ipv4_getsockname.c:82` does.



##########
net/can/can_sendmsg_buffered.c:
##########
@@ -480,7 +475,7 @@ void can_sendbuffer_notify(FAR struct can_conn_s *conn)
   int val = 0;
 
   nxsem_get_value(&conn->sndsem, &val);
-  if (val < 0)
+  if (val < 1)

Review Comment:
   With `val < 1` the semaphore stays at 1 after a completion, and the check at 
line 247 is an `if`, so the next send queues past `conn->sndbufs`.
   
   Fix: make the check at line 247 a `while` loop around 
`conn_dev_sem_timedwait()`.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to