[GitHub] [nuttx] xiaoxiang781216 commented on a diff in pull request #8361: net/devif: check the net device before use

["xiaoxiang781216 (via GitHub)" <gi...@apache.org>]

xiaoxiang781216 commented on code in PR #8361:
URL: https://github.com/apache/nuttx/pull/8361#discussion_r1091589847


##########
net/devif/devif_send.c:
##########
@@ -68,47 +69,50 @@
 void devif_send(FAR struct net_driver_s *dev, FAR const void *buf,
                 int len, unsigned int offset)
 {
-#ifndef CONFIG_NET_IPFRAG
-  unsigned int limit = NETDEV_PKTSIZE(dev) -
-                       NET_LL_HDRLEN(dev) - offset;
+  int ret;
 
-  if (dev == NULL || len == 0 || len > limit)
+  if (dev == NULL)
     {
-      nerr("ERROR: devif_send fail: %p, sndlen: %u, pktlen: %u\n",
-           dev, len, limit);
-      return;
+      ret = -ENODEV;
+      goto errout;
     }
-#else
-  if (dev == NULL || len == 0)
+
+  if (len == 0)
+    {
+      ret = -EINVAL;
+      goto errout;
+    }
+
+#ifndef CONFIG_NET_IPFRAG
+  if (len > NETDEV_PKTSIZE(dev) - NET_LL_HDRLEN(dev) - offset)
     {
-      nerr("ERROR: devif_send fail: %p, sndlen: %u\n", dev, len);
-      return;
+      ret = -EMSGSIZE;
+      goto errout;
     }
 #endif
 
-  /* Copy in iob to target device buffer */
+  /* Append the send buffer after device buffer */
 
-  if (len <= iob_navail(false) * CONFIG_IOB_BUFSIZE)
+  if (len > iob_navail(false) * CONFIG_IOB_BUFSIZE &&
+      netdev_iob_prepare(dev, false, 0) != OK)
     {
-      /* Prepare device buffer before poll callback */
+      ret = -ENOMEM;
+      goto errout;
+    }
 
-      if (netdev_iob_prepare(dev, false, 0) != OK)
-        {
-          return;
-        }
+  /* Prepare device buffer before poll callback */
 
-      iob_update_pktlen(dev->d_iob, offset);
+  iob_update_pktlen(dev->d_iob, offset);
 
-      dev->d_sndlen = iob_trycopyin(dev->d_iob, buf, len, offset, false);
-    }
-  else
+  ret = iob_trycopyin(dev->d_iob, buf, len, offset, false);
+  if (ret != len)
     {
-      dev->d_sndlen = 0;
+      netdev_iob_release(dev);
     }
 
-  if (dev->d_sndlen != len)

Review Comment:
   let's return here to avoid the check(ret < 0) after errout



##########
net/devif/devif_iobsend.c:
##########
@@ -56,67 +57,59 @@ void devif_iob_send(FAR struct net_driver_s *dev, FAR struct iob_s *iob,
                     unsigned int len, unsigned int offset,
                     unsigned int target_offset)
 {
-#ifndef CONFIG_NET_IPFRAG
-  unsigned int limit = NETDEV_PKTSIZE(dev) -
-                       NET_LL_HDRLEN(dev) - target_offset;
+  int ret;
 
-  if (dev == NULL || len == 0 || len > limit)
-#else
-  if (dev == NULL || len == 0)
-#endif
+  if (dev == NULL)
     {
-      if (dev->d_iob == NULL)
-        {
-          iob_free_chain(iob);
-        }
+      ret = -ENODEV;
+      goto errout;
+    }
+
+  if (len == 0)
+    {
+      ret = -EINVAL;
+      goto errout;
+    }
 
 #ifndef CONFIG_NET_IPFRAG
-      nerr("devif_iob_send error, %p, send len: %u, limit len: %u\n",
-           dev, len, limit);
-#else
-      nerr("devif_iob_send error, %p, send len: %u\n", dev, len);
-#endif
-      return;
+  if (len > NETDEV_PKTSIZE(dev) - NET_LL_HDRLEN(dev) - target_offset)
+    {
+      ret = -EMSGSIZE;
+      goto errout;
     }
+#endif
 
   /* Append the send buffer after device buffer */
 
-  if (dev->d_iob != NULL)
+  if (len > iob_navail(false) * CONFIG_IOB_BUFSIZE)
     {
-      dev->d_sndlen = 0;
-
-      if (len > iob_navail(false) * CONFIG_IOB_BUFSIZE)
-        {
-          return;
-        }
-
-      /* Clone the iob to target device buffer */
+      ret = -ENOMEM;
+      goto errout;
+    }
 
-      if (iob_clone_partial(iob, len, offset, dev->d_iob,
-                            target_offset, false, false) != OK)
-        {
-          netdev_iob_release(dev);
-          nerr("devif_iob_send error, not enough iob entries, "
-              "send len: %u\n", len);
-          return;
-        }
+  /* Clone the iob to target device buffer */
 
-      dev->d_sndlen = len;
-    }
-  else
+  ret = iob_clone_partial(iob, len, offset, dev->d_iob,
+                          target_offset, false, false);
+  if (ret != OK)
     {
-      /* Send the iob directly if no device buffer */
-
-      dev->d_iob    = iob;
-      dev->d_sndlen = len;
-      dev->d_buf    = NETLLBUF;
+      netdev_iob_release(dev);
+      goto errout;
     }
 
+  dev->d_sndlen = len;
+
 #ifdef CONFIG_NET_TCP_WRBUFFER_DUMP
   /* Dump the outgoing device buffer */
 
   lib_dumpbuffer("devif_iob_send", dev->d_appdata, len);
 #endif
+
+errout:

Review Comment:
   let's return here to avoid the check(ret < 0) after errout



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