You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/09/06 14:37:31 UTC
[GitHub] [incubator-nuttx] anchao opened a new pull request, #7020: net/netdev: simplify handling of netdev ifr ioctl()
anchao opened a new pull request, #7020:
URL: https://github.com/apache/incubator-nuttx/pull/7020
## Summary
net/netdev: simplify handling of netdev ifr ioctl()
1. call netdev_ifr_dev() only once
2. unify error code of ENODEV
Signed-off-by: chao an <an...@xiaomi.com>
## Impact
N/A
## Testing
netdev_ifr_ioctl
--
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
[GitHub] [incubator-nuttx] masayuki2009 commented on pull request #7020: net/netdev: simplify handling of netdev ifr ioctl()
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on PR #7020:
URL: https://github.com/apache/incubator-nuttx/pull/7020#issuecomment-1240206561
@anchao
I noticed that the renew command does not work now.
```
/home/ishikawa/opensource/QEMU/qemu-6.2/build/arm-softmmu/qemu-system-arm -net nic,model=stellaris -net user,hostfwd=tcp:127.0.0.1:20023-10.0.2.15:23,hostfwd=tcp:127.0.0.1:20021-10.0.2.15:21,hostfwd=tcp:127.0.0.1:25001-10.0.2.15:5001 -M lm3s6965evb -kernel ./nuttx/nuttx -nographic
NuttShell (NSH) NuttX-10.4.0
nsh> uname -a
NuttX 10.4.0 fd53db56b6 Sep 8 2022 13:25:03 arm lm3s6965-ek
nsh> ps
PID GROUP PRI POLICY TYPE NPX STATE EVENT SIGMASK STACK USED FILLED COMMAND
0 0 0 FIFO Kthread N-- Ready 00000000 001000 000380 38.0% Idle Task
1 1 224 RR Kthread --- Waiting Semaphore 00000000 001992 000276 13.8% hpwork 0x2000095c
2 2 60 RR Kthread --- Waiting Semaphore 00000000 001992 000276 13.8% lpwork 0x2000096c
3 3 100 RR Task --- Running 00000000 002000 001208 60.4% nsh_main
4 4 100 RR Task --- Waiting Semaphore 00000000 001984 000500 25.2% Telnet daemon 0x20006720
nsh> free
total used free largest nused nfree
Umem: 50208 15408 34800 34800 60 1
nsh> ifconfig
eth0 Link encap:Ethernet HWaddr 00:e0:de:ad:be:ef at UP
inet addr:10.0.0.2 DRaddr:10.0.0.1 Mask:255.255.255.0
IPv4 TCP UDP ICMP
Received 0000 0000 0000 0000
Dropped 0000 0000 0000 0000
IPv4 VHL: 0000 Frg: 0000
Checksum 0000 0000 0000 ----
TCP ACK: 0000 SYN: 0000
RST: 0000 0000
Type 0000 ---- ---- 0000
Sent 0000 0000 0000 0000
Rexmit ---- 0000 ---- ----
nsh> renew eth0
```
--
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
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #7020: net/netdev: simplify handling of netdev ifr ioctl()
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #7020:
URL: https://github.com/apache/incubator-nuttx/pull/7020#discussion_r964349288
##########
net/netdev/netdev_ioctl.c:
##########
@@ -636,235 +700,201 @@ static FAR struct net_driver_s *netdev_ifr_dev(FAR struct ifreq *req)
static int netdev_ifr_ioctl(FAR struct socket *psock, int cmd,
FAR struct ifreq *req)
{
- FAR struct net_driver_s *dev;
- int ret = -EINVAL;
+ FAR struct net_driver_s *dev = NULL;
+ int ret = OK;
ninfo("cmd: %d\n", cmd);
net_lock();
- /* Execute the command */
+ /* Execute commands that do not need ifr_name or lifr_name */
switch (cmd)
{
+ case SIOCGIFCOUNT: /* Get number of devices */
+ {
Review Comment:
let's remove all unnecessary {}
--
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
[GitHub] [incubator-nuttx] anchao commented on a diff in pull request #7020: net/netdev: simplify handling of netdev ifr ioctl()
Posted by GitBox <gi...@apache.org>.
anchao commented on code in PR #7020:
URL: https://github.com/apache/incubator-nuttx/pull/7020#discussion_r964351799
##########
net/netdev/netdev_ioctl.c:
##########
@@ -636,235 +700,201 @@ static FAR struct net_driver_s *netdev_ifr_dev(FAR struct ifreq *req)
static int netdev_ifr_ioctl(FAR struct socket *psock, int cmd,
FAR struct ifreq *req)
{
- FAR struct net_driver_s *dev;
- int ret = -EINVAL;
+ FAR struct net_driver_s *dev = NULL;
+ int ret = OK;
ninfo("cmd: %d\n", cmd);
net_lock();
- /* Execute the command */
+ /* Execute commands that do not need ifr_name or lifr_name */
switch (cmd)
{
+ case SIOCGIFCOUNT: /* Get number of devices */
+ {
Review Comment:
Done
--
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
[GitHub] [incubator-nuttx] anchao commented on a diff in pull request #7020: net/netdev: simplify handling of netdev ifr ioctl()
Posted by GitBox <gi...@apache.org>.
anchao commented on code in PR #7020:
URL: https://github.com/apache/incubator-nuttx/pull/7020#discussion_r964337755
##########
net/netdev/netdev_ioctl.c:
##########
@@ -636,235 +636,204 @@ static FAR struct net_driver_s *netdev_ifr_dev(FAR struct ifreq *req)
static int netdev_ifr_ioctl(FAR struct socket *psock, int cmd,
FAR struct ifreq *req)
{
- FAR struct net_driver_s *dev;
- int ret = -EINVAL;
+ FAR struct net_driver_s *dev = NULL;
+ FAR struct net_driver_s *tmpdev;
+ ssize_t arglen;
+ int ret;
ninfo("cmd: %d\n", cmd);
net_lock();
- /* Execute the command */
+ /* Execute the command without ifr_name or lifr_name */
switch (cmd)
{
+ case SIOCGIFCOUNT: /* Get number of devices */
+ {
+ req->ifr_count = netdev_count();
+ ret = OK;
+ }
+ break;
+
#ifdef CONFIG_NET_IPv4
- case SIOCGIFADDR: /* Get IP address */
+ case SIOCGIFCONF: /* Return an interface list (IPv4) */
{
- dev = netdev_ifr_dev(req);
- if (dev)
- {
- ioctl_get_ipv4addr(&req->ifr_addr, dev->d_ipaddr);
- ret = OK;
- }
+ ret = netdev_ipv4_ifconf((FAR struct ifconf *)req);
}
break;
#endif
-#ifdef CONFIG_NET_IPv4
- case SIOCSIFADDR: /* Set IP address */
+#ifdef CONFIG_NET_IPv6
+ case SIOCGLIFCONF: /* Return an interface list (IPv6) */
{
- dev = netdev_ifr_dev(req);
- if (dev)
- {
- ioctl_set_ipv4addr(&dev->d_ipaddr, &req->ifr_addr);
- ret = OK;
- }
+ ret = netdev_ipv6_ifconf((FAR struct lifconf *)req);
}
break;
#endif
-#ifdef CONFIG_NET_IPv4
- case SIOCGIFDSTADDR: /* Get P-to-P address */
+#ifdef CONFIG_NETDEV_IFINDEX
+ case SIOCGIFNAME: /* Get interface name */
{
- dev = netdev_ifr_dev(req);
- if (dev)
+ tmpdev = netdev_findbyindex(req->ifr_ifindex);
+ if (tmpdev != NULL)
{
- ioctl_get_ipv4addr(&req->ifr_dstaddr, dev->d_draddr);
+ strlcpy(req->ifr_name, tmpdev->d_ifname, IFNAMSIZ);
ret = OK;
Review Comment:
done
--
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
[GitHub] [incubator-nuttx] hartmannathan commented on a diff in pull request #7020: net/netdev: simplify handling of netdev ifr ioctl()
Posted by GitBox <gi...@apache.org>.
hartmannathan commented on code in PR #7020:
URL: https://github.com/apache/incubator-nuttx/pull/7020#discussion_r964255066
##########
net/netdev/netdev_ioctl.c:
##########
@@ -636,235 +636,204 @@ static FAR struct net_driver_s *netdev_ifr_dev(FAR struct ifreq *req)
static int netdev_ifr_ioctl(FAR struct socket *psock, int cmd,
FAR struct ifreq *req)
{
- FAR struct net_driver_s *dev;
- int ret = -EINVAL;
+ FAR struct net_driver_s *dev = NULL;
+ FAR struct net_driver_s *tmpdev;
+ ssize_t arglen;
+ int ret;
ninfo("cmd: %d\n", cmd);
net_lock();
- /* Execute the command */
+ /* Execute the command without ifr_name or lifr_name */
switch (cmd)
{
+ case SIOCGIFCOUNT: /* Get number of devices */
+ {
+ req->ifr_count = netdev_count();
+ ret = OK;
+ }
+ break;
+
#ifdef CONFIG_NET_IPv4
- case SIOCGIFADDR: /* Get IP address */
+ case SIOCGIFCONF: /* Return an interface list (IPv4) */
{
- dev = netdev_ifr_dev(req);
- if (dev)
- {
- ioctl_get_ipv4addr(&req->ifr_addr, dev->d_ipaddr);
- ret = OK;
- }
+ ret = netdev_ipv4_ifconf((FAR struct ifconf *)req);
}
break;
#endif
-#ifdef CONFIG_NET_IPv4
- case SIOCSIFADDR: /* Set IP address */
+#ifdef CONFIG_NET_IPv6
+ case SIOCGLIFCONF: /* Return an interface list (IPv6) */
{
- dev = netdev_ifr_dev(req);
- if (dev)
- {
- ioctl_set_ipv4addr(&dev->d_ipaddr, &req->ifr_addr);
- ret = OK;
- }
+ ret = netdev_ipv6_ifconf((FAR struct lifconf *)req);
}
break;
#endif
-#ifdef CONFIG_NET_IPv4
- case SIOCGIFDSTADDR: /* Get P-to-P address */
+#ifdef CONFIG_NETDEV_IFINDEX
+ case SIOCGIFNAME: /* Get interface name */
{
- dev = netdev_ifr_dev(req);
- if (dev)
+ tmpdev = netdev_findbyindex(req->ifr_ifindex);
Review Comment:
Hmm... compiler is complaining tmpdev is unused?
--
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
[GitHub] [incubator-nuttx] anchao commented on a diff in pull request #7020: net/netdev: simplify handling of netdev ifr ioctl()
Posted by GitBox <gi...@apache.org>.
anchao commented on code in PR #7020:
URL: https://github.com/apache/incubator-nuttx/pull/7020#discussion_r964337848
##########
net/netdev/netdev_ioctl.c:
##########
@@ -636,235 +636,204 @@ static FAR struct net_driver_s *netdev_ifr_dev(FAR struct ifreq *req)
static int netdev_ifr_ioctl(FAR struct socket *psock, int cmd,
FAR struct ifreq *req)
{
- FAR struct net_driver_s *dev;
- int ret = -EINVAL;
+ FAR struct net_driver_s *dev = NULL;
+ FAR struct net_driver_s *tmpdev;
+ ssize_t arglen;
+ int ret;
ninfo("cmd: %d\n", cmd);
net_lock();
- /* Execute the command */
+ /* Execute the command without ifr_name or lifr_name */
switch (cmd)
{
+ case SIOCGIFCOUNT: /* Get number of devices */
+ {
+ req->ifr_count = netdev_count();
+ ret = OK;
+ }
+ break;
+
#ifdef CONFIG_NET_IPv4
- case SIOCGIFADDR: /* Get IP address */
+ case SIOCGIFCONF: /* Return an interface list (IPv4) */
{
- dev = netdev_ifr_dev(req);
- if (dev)
- {
- ioctl_get_ipv4addr(&req->ifr_addr, dev->d_ipaddr);
- ret = OK;
- }
+ ret = netdev_ipv4_ifconf((FAR struct ifconf *)req);
}
break;
#endif
-#ifdef CONFIG_NET_IPv4
- case SIOCSIFADDR: /* Set IP address */
+#ifdef CONFIG_NET_IPv6
+ case SIOCGLIFCONF: /* Return an interface list (IPv6) */
{
- dev = netdev_ifr_dev(req);
- if (dev)
- {
- ioctl_set_ipv4addr(&dev->d_ipaddr, &req->ifr_addr);
- ret = OK;
- }
+ ret = netdev_ipv6_ifconf((FAR struct lifconf *)req);
}
break;
#endif
-#ifdef CONFIG_NET_IPv4
- case SIOCGIFDSTADDR: /* Get P-to-P address */
+#ifdef CONFIG_NETDEV_IFINDEX
+ case SIOCGIFNAME: /* Get interface name */
{
- dev = netdev_ifr_dev(req);
- if (dev)
+ tmpdev = netdev_findbyindex(req->ifr_ifindex);
Review Comment:
Done
##########
net/netdev/netdev_ioctl.c:
##########
@@ -636,235 +636,204 @@ static FAR struct net_driver_s *netdev_ifr_dev(FAR struct ifreq *req)
static int netdev_ifr_ioctl(FAR struct socket *psock, int cmd,
FAR struct ifreq *req)
{
- FAR struct net_driver_s *dev;
- int ret = -EINVAL;
+ FAR struct net_driver_s *dev = NULL;
+ FAR struct net_driver_s *tmpdev;
+ ssize_t arglen;
+ int ret;
ninfo("cmd: %d\n", cmd);
net_lock();
- /* Execute the command */
+ /* Execute the command without ifr_name or lifr_name */
switch (cmd)
{
+ case SIOCGIFCOUNT: /* Get number of devices */
+ {
+ req->ifr_count = netdev_count();
+ ret = OK;
Review Comment:
done
--
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
[GitHub] [incubator-nuttx] anchao commented on a diff in pull request #7020: net/netdev: simplify handling of netdev ifr ioctl()
Posted by GitBox <gi...@apache.org>.
anchao commented on code in PR #7020:
URL: https://github.com/apache/incubator-nuttx/pull/7020#discussion_r964373476
##########
net/netdev/netdev_ioctl.c:
##########
@@ -636,279 +700,183 @@ static FAR struct net_driver_s *netdev_ifr_dev(FAR struct ifreq *req)
static int netdev_ifr_ioctl(FAR struct socket *psock, int cmd,
FAR struct ifreq *req)
{
- FAR struct net_driver_s *dev;
- int ret = -EINVAL;
+ FAR struct net_driver_s *dev = NULL;
+ int ret = OK;
ninfo("cmd: %d\n", cmd);
net_lock();
- /* Execute the command */
+ /* Execute commands that do not need ifr_name or lifr_name */
switch (cmd)
{
+ case SIOCGIFCOUNT: /* Get number of devices */
+ req->ifr_count = netdev_count();
+ break;
+
#ifdef CONFIG_NET_IPv4
- case SIOCGIFADDR: /* Get IP address */
- {
- dev = netdev_ifr_dev(req);
- if (dev)
- {
- ioctl_get_ipv4addr(&req->ifr_addr, dev->d_ipaddr);
- ret = OK;
- }
- }
+ case SIOCGIFCONF: /* Return an interface list (IPv4) */
+ ret = netdev_ipv4_ifconf((FAR struct ifconf *)req);
break;
#endif
-#ifdef CONFIG_NET_IPv4
- case SIOCSIFADDR: /* Set IP address */
- {
- dev = netdev_ifr_dev(req);
- if (dev)
- {
- ioctl_set_ipv4addr(&dev->d_ipaddr, &req->ifr_addr);
- ret = OK;
- }
- }
+#ifdef CONFIG_NET_IPv6
+ case SIOCGLIFCONF: /* Return an interface list (IPv6) */
+ ret = netdev_ipv6_ifconf((FAR struct lifconf *)req);
break;
#endif
-#ifdef CONFIG_NET_IPv4
- case SIOCGIFDSTADDR: /* Get P-to-P address */
+#ifdef CONFIG_NETDEV_IFINDEX
+ case SIOCGIFNAME: /* Get interface name */
{
- dev = netdev_ifr_dev(req);
- if (dev)
+ FAR struct net_driver_s *tmpdev;
+ tmpdev = netdev_findbyindex(req->ifr_ifindex);
+ if (tmpdev != NULL)
+ {
+ strlcpy(req->ifr_name, tmpdev->d_ifname, IFNAMSIZ);
+ }
+ else
{
- ioctl_get_ipv4addr(&req->ifr_dstaddr, dev->d_draddr);
- ret = OK;
+ ret = -ENODEV;
}
}
break;
#endif
-
-#ifdef CONFIG_NET_IPv4
- case SIOCSIFDSTADDR: /* Set P-to-P address */
+ default:
{
- dev = netdev_ifr_dev(req);
- if (dev)
+ if (net_ioctl_ifreq_arglen(cmd) > 0)
{
- ioctl_set_ipv4addr(&dev->d_draddr, &req->ifr_dstaddr);
- ret = OK;
+ dev = netdev_ifr_dev(req);
+ if (dev == NULL)
+ {
+ ret = -ENODEV;
+ }
+ }
+ else
+ {
+ ret = -ENOTTY;
}
}
break;
-#endif
+ }
+
+ if (dev == NULL)
+ {
+ return ret;
+ }
+
+ /* Execute commands that need ifr_name or lifr_name */
+ switch (cmd)
+ {
#ifdef CONFIG_NET_IPv4
+ case SIOCGIFADDR: /* Get IP address */
+ ioctl_get_ipv4addr(&req->ifr_addr, dev->d_ipaddr);
+ break;
+
+ case SIOCSIFADDR: /* Set IP address */
+ ioctl_set_ipv4addr(&dev->d_ipaddr, &req->ifr_addr);
+ break;
+
+ case SIOCGIFDSTADDR: /* Get P-to-P address */
+ ioctl_get_ipv4addr(&req->ifr_dstaddr, dev->d_draddr);
+ break;
+
+ case SIOCSIFDSTADDR: /* Set P-to-P address */
+ ioctl_set_ipv4addr(&dev->d_draddr, &req->ifr_dstaddr);
+ break;
+
case SIOCGIFBRDADDR: /* Get broadcast IP address */
- {
- dev = netdev_ifr_dev(req);
- if (dev)
- {
- ioctl_get_ipv4broadcast(&req->ifr_broadaddr, dev->d_ipaddr,
- dev->d_netmask);
- ret = OK;
- }
- }
+ ioctl_get_ipv4broadcast(&req->ifr_broadaddr, dev->d_ipaddr,
+ dev->d_netmask);
break;
-#endif
-#ifdef CONFIG_NET_IPv4
case SIOCSIFBRDADDR: /* Set broadcast IP address */
- {
- ret = -ENOSYS;
- }
+ ret = -ENOSYS;
break;
-#endif
-#ifdef CONFIG_NET_IPv4
case SIOCGIFNETMASK: /* Get network mask */
- {
- dev = netdev_ifr_dev(req);
- if (dev)
- {
- ioctl_get_ipv4addr(&req->ifr_addr, dev->d_netmask);
- ret = OK;
- }
- }
+ ioctl_get_ipv4addr(&req->ifr_addr, dev->d_netmask);
break;
-#endif
-#ifdef CONFIG_NET_IPv4
case SIOCSIFNETMASK: /* Set network mask */
- {
- dev = netdev_ifr_dev(req);
- if (dev)
- {
- ioctl_set_ipv4addr(&dev->d_netmask, &req->ifr_addr);
- ret = OK;
- }
- }
+ ioctl_set_ipv4addr(&dev->d_netmask, &req->ifr_addr);
break;
#endif
#ifdef CONFIG_NET_IPv6
case SIOCGLIFADDR: /* Get IP address */
- {
- dev = netdev_ifr_dev(req);
- if (dev)
- {
- FAR struct lifreq *lreq = (FAR struct lifreq *)req;
-
- ioctl_get_ipv6addr(&lreq->lifr_addr, dev->d_ipv6addr);
- ret = OK;
- }
- }
+ FAR struct lifreq *lreq = (FAR struct lifreq *)req;
Review Comment:
Done
##########
net/netdev/netdev_ioctl.c:
##########
@@ -636,279 +700,183 @@ static FAR struct net_driver_s *netdev_ifr_dev(FAR struct ifreq *req)
static int netdev_ifr_ioctl(FAR struct socket *psock, int cmd,
FAR struct ifreq *req)
{
- FAR struct net_driver_s *dev;
- int ret = -EINVAL;
+ FAR struct net_driver_s *dev = NULL;
+ int ret = OK;
ninfo("cmd: %d\n", cmd);
net_lock();
- /* Execute the command */
+ /* Execute commands that do not need ifr_name or lifr_name */
switch (cmd)
{
+ case SIOCGIFCOUNT: /* Get number of devices */
+ req->ifr_count = netdev_count();
+ break;
+
#ifdef CONFIG_NET_IPv4
- case SIOCGIFADDR: /* Get IP address */
- {
- dev = netdev_ifr_dev(req);
- if (dev)
- {
- ioctl_get_ipv4addr(&req->ifr_addr, dev->d_ipaddr);
- ret = OK;
- }
- }
+ case SIOCGIFCONF: /* Return an interface list (IPv4) */
+ ret = netdev_ipv4_ifconf((FAR struct ifconf *)req);
break;
#endif
-#ifdef CONFIG_NET_IPv4
- case SIOCSIFADDR: /* Set IP address */
- {
- dev = netdev_ifr_dev(req);
- if (dev)
- {
- ioctl_set_ipv4addr(&dev->d_ipaddr, &req->ifr_addr);
- ret = OK;
- }
- }
+#ifdef CONFIG_NET_IPv6
+ case SIOCGLIFCONF: /* Return an interface list (IPv6) */
+ ret = netdev_ipv6_ifconf((FAR struct lifconf *)req);
break;
#endif
-#ifdef CONFIG_NET_IPv4
- case SIOCGIFDSTADDR: /* Get P-to-P address */
+#ifdef CONFIG_NETDEV_IFINDEX
+ case SIOCGIFNAME: /* Get interface name */
{
- dev = netdev_ifr_dev(req);
- if (dev)
+ FAR struct net_driver_s *tmpdev;
+ tmpdev = netdev_findbyindex(req->ifr_ifindex);
+ if (tmpdev != NULL)
+ {
+ strlcpy(req->ifr_name, tmpdev->d_ifname, IFNAMSIZ);
+ }
+ else
{
- ioctl_get_ipv4addr(&req->ifr_dstaddr, dev->d_draddr);
- ret = OK;
+ ret = -ENODEV;
}
}
break;
#endif
-
-#ifdef CONFIG_NET_IPv4
- case SIOCSIFDSTADDR: /* Set P-to-P address */
+ default:
{
- dev = netdev_ifr_dev(req);
- if (dev)
+ if (net_ioctl_ifreq_arglen(cmd) > 0)
{
- ioctl_set_ipv4addr(&dev->d_draddr, &req->ifr_dstaddr);
- ret = OK;
+ dev = netdev_ifr_dev(req);
+ if (dev == NULL)
+ {
+ ret = -ENODEV;
+ }
+ }
+ else
+ {
+ ret = -ENOTTY;
}
}
break;
-#endif
+ }
+
+ if (dev == NULL)
+ {
+ return ret;
+ }
+
+ /* Execute commands that need ifr_name or lifr_name */
+ switch (cmd)
+ {
#ifdef CONFIG_NET_IPv4
+ case SIOCGIFADDR: /* Get IP address */
+ ioctl_get_ipv4addr(&req->ifr_addr, dev->d_ipaddr);
+ break;
+
+ case SIOCSIFADDR: /* Set IP address */
+ ioctl_set_ipv4addr(&dev->d_ipaddr, &req->ifr_addr);
+ break;
+
+ case SIOCGIFDSTADDR: /* Get P-to-P address */
+ ioctl_get_ipv4addr(&req->ifr_dstaddr, dev->d_draddr);
+ break;
+
+ case SIOCSIFDSTADDR: /* Set P-to-P address */
+ ioctl_set_ipv4addr(&dev->d_draddr, &req->ifr_dstaddr);
+ break;
+
case SIOCGIFBRDADDR: /* Get broadcast IP address */
- {
- dev = netdev_ifr_dev(req);
- if (dev)
- {
- ioctl_get_ipv4broadcast(&req->ifr_broadaddr, dev->d_ipaddr,
- dev->d_netmask);
- ret = OK;
- }
- }
+ ioctl_get_ipv4broadcast(&req->ifr_broadaddr, dev->d_ipaddr,
+ dev->d_netmask);
break;
-#endif
-#ifdef CONFIG_NET_IPv4
case SIOCSIFBRDADDR: /* Set broadcast IP address */
- {
- ret = -ENOSYS;
- }
+ ret = -ENOSYS;
break;
-#endif
-#ifdef CONFIG_NET_IPv4
case SIOCGIFNETMASK: /* Get network mask */
- {
- dev = netdev_ifr_dev(req);
- if (dev)
- {
- ioctl_get_ipv4addr(&req->ifr_addr, dev->d_netmask);
- ret = OK;
- }
- }
+ ioctl_get_ipv4addr(&req->ifr_addr, dev->d_netmask);
break;
-#endif
-#ifdef CONFIG_NET_IPv4
case SIOCSIFNETMASK: /* Set network mask */
- {
- dev = netdev_ifr_dev(req);
- if (dev)
- {
- ioctl_set_ipv4addr(&dev->d_netmask, &req->ifr_addr);
- ret = OK;
- }
- }
+ ioctl_set_ipv4addr(&dev->d_netmask, &req->ifr_addr);
break;
#endif
#ifdef CONFIG_NET_IPv6
case SIOCGLIFADDR: /* Get IP address */
- {
- dev = netdev_ifr_dev(req);
- if (dev)
- {
- FAR struct lifreq *lreq = (FAR struct lifreq *)req;
-
- ioctl_get_ipv6addr(&lreq->lifr_addr, dev->d_ipv6addr);
- ret = OK;
- }
- }
+ FAR struct lifreq *lreq = (FAR struct lifreq *)req;
+ ioctl_get_ipv6addr(&lreq->lifr_addr, dev->d_ipv6addr);
break;
-#endif
-#ifdef CONFIG_NET_IPv6
case SIOCSLIFADDR: /* Set IP address */
- {
- dev = netdev_ifr_dev(req);
- if (dev)
- {
- FAR struct lifreq *lreq = (FAR struct lifreq *)req;
-
- ioctl_set_ipv6addr(dev->d_ipv6addr, &lreq->lifr_addr);
- ret = OK;
- }
- }
+ FAR struct lifreq *lreq = (FAR struct lifreq *)req;
Review Comment:
Done
--
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
[GitHub] [incubator-nuttx] anchao commented on a diff in pull request #7020: net/netdev: simplify handling of netdev ifr ioctl()
Posted by GitBox <gi...@apache.org>.
anchao commented on code in PR #7020:
URL: https://github.com/apache/incubator-nuttx/pull/7020#discussion_r964552463
##########
net/netdev/netdev_ioctl.c:
##########
@@ -616,6 +616,70 @@ static FAR struct net_driver_s *netdev_ifr_dev(FAR struct ifreq *req)
return NULL;
}
+/****************************************************************************
+ * Name: net_ioctl_ifreq_arglen
+ *
+ * Description:
+ * Calculate the ioctl argument buffer length of ifreq.
+ *
+ * Input Parameters:
+ *
+ * cmd The ioctl command
+ *
+ * Returned Value:
+ * The argument buffer length, or error code.
+ *
+ ****************************************************************************/
+
+static ssize_t net_ioctl_ifreq_arglen(int cmd)
+{
+ switch (cmd)
+ {
+ case SIOCGIFADDR:
+ case SIOCSIFADDR:
+ case SIOCGIFDSTADDR:
+ case SIOCSIFDSTADDR:
+ case SIOCGIFBRDADDR:
+ case SIOCSIFBRDADDR:
+ case SIOCGIFNETMASK:
+ case SIOCSIFNETMASK:
+ case SIOCGIFMTU:
+ case SIOCGIFHWADDR:
+ case SIOCSIFHWADDR:
+ case SIOCDIFADDR:
+ case SIOCGIFCOUNT:
+ case SIOCSIFFLAGS:
+ case SIOCGIFFLAGS:
+ case SIOCMIINOTIFY:
+ case SIOCGMIIPHY:
+ case SIOCGMIIREG:
+ case SIOCSMIIREG:
+ case SIOCGCANBITRATE:
+ case SIOCSCANBITRATE:
+ case SIOCACANEXTFILTER:
+ case SIOCDCANEXTFILTER:
+ case SIOCACANSTDFILTER:
+ case SIOCDCANSTDFILTER:
+ case SIOCGIFNAME:
+ case SIOCGIFINDEX:
+ return sizeof(struct ifreq);
+
+ case SIOCGLIFADDR:
+ case SIOCSLIFADDR:
+ case SIOCGLIFDSTADDR:
+ case SIOCSLIFDSTADDR:
+ case SIOCGLIFBRDADDR:
+ case SIOCSLIFBRDADDR:
+ case SIOCGLIFNETMASK:
+ case SIOCSLIFNETMASK:
+ case SIOCGLIFMTU:
+ case SIOCIFAUTOCONF:
+ return sizeof(struct lifreq);
+ }
Review Comment:
Done!
--
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
[GitHub] [incubator-nuttx] anchao commented on pull request #7020: net/netdev: simplify handling of netdev ifr ioctl()
Posted by GitBox <gi...@apache.org>.
anchao commented on PR #7020:
URL: https://github.com/apache/incubator-nuttx/pull/7020#issuecomment-1238245302
@hartmannathan
https://github.com/apache/incubator-nuttx/pull/7014#pullrequestreview-1097567649
--
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
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #7020: net/netdev: simplify handling of netdev ifr ioctl()
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #7020:
URL: https://github.com/apache/incubator-nuttx/pull/7020#discussion_r964006952
##########
net/netdev/netdev_ioctl.c:
##########
@@ -636,235 +636,204 @@ static FAR struct net_driver_s *netdev_ifr_dev(FAR struct ifreq *req)
static int netdev_ifr_ioctl(FAR struct socket *psock, int cmd,
FAR struct ifreq *req)
{
- FAR struct net_driver_s *dev;
- int ret = -EINVAL;
+ FAR struct net_driver_s *dev = NULL;
+ FAR struct net_driver_s *tmpdev;
+ ssize_t arglen;
+ int ret;
ninfo("cmd: %d\n", cmd);
net_lock();
- /* Execute the command */
+ /* Execute the command without ifr_name or lifr_name */
switch (cmd)
{
+ case SIOCGIFCOUNT: /* Get number of devices */
+ {
+ req->ifr_count = netdev_count();
+ ret = OK;
Review Comment:
let's set ret = OK at line 642 and remove this line
##########
net/netdev/netdev_ioctl.c:
##########
@@ -636,235 +636,204 @@ static FAR struct net_driver_s *netdev_ifr_dev(FAR struct ifreq *req)
static int netdev_ifr_ioctl(FAR struct socket *psock, int cmd,
FAR struct ifreq *req)
{
- FAR struct net_driver_s *dev;
- int ret = -EINVAL;
+ FAR struct net_driver_s *dev = NULL;
+ FAR struct net_driver_s *tmpdev;
+ ssize_t arglen;
+ int ret;
ninfo("cmd: %d\n", cmd);
net_lock();
- /* Execute the command */
+ /* Execute the command without ifr_name or lifr_name */
switch (cmd)
{
+ case SIOCGIFCOUNT: /* Get number of devices */
+ {
+ req->ifr_count = netdev_count();
+ ret = OK;
+ }
+ break;
+
#ifdef CONFIG_NET_IPv4
- case SIOCGIFADDR: /* Get IP address */
+ case SIOCGIFCONF: /* Return an interface list (IPv4) */
{
- dev = netdev_ifr_dev(req);
- if (dev)
- {
- ioctl_get_ipv4addr(&req->ifr_addr, dev->d_ipaddr);
- ret = OK;
- }
+ ret = netdev_ipv4_ifconf((FAR struct ifconf *)req);
}
break;
#endif
-#ifdef CONFIG_NET_IPv4
- case SIOCSIFADDR: /* Set IP address */
+#ifdef CONFIG_NET_IPv6
+ case SIOCGLIFCONF: /* Return an interface list (IPv6) */
{
- dev = netdev_ifr_dev(req);
- if (dev)
- {
- ioctl_set_ipv4addr(&dev->d_ipaddr, &req->ifr_addr);
- ret = OK;
- }
+ ret = netdev_ipv6_ifconf((FAR struct lifconf *)req);
}
break;
#endif
-#ifdef CONFIG_NET_IPv4
- case SIOCGIFDSTADDR: /* Get P-to-P address */
+#ifdef CONFIG_NETDEV_IFINDEX
+ case SIOCGIFNAME: /* Get interface name */
{
- dev = netdev_ifr_dev(req);
- if (dev)
+ tmpdev = netdev_findbyindex(req->ifr_ifindex);
+ if (tmpdev != NULL)
{
- ioctl_get_ipv4addr(&req->ifr_dstaddr, dev->d_draddr);
+ strlcpy(req->ifr_name, tmpdev->d_ifname, IFNAMSIZ);
ret = OK;
Review Comment:
remove
##########
net/netdev/netdev_ioctl.c:
##########
@@ -636,235 +636,204 @@ static FAR struct net_driver_s *netdev_ifr_dev(FAR struct ifreq *req)
static int netdev_ifr_ioctl(FAR struct socket *psock, int cmd,
FAR struct ifreq *req)
{
- FAR struct net_driver_s *dev;
- int ret = -EINVAL;
+ FAR struct net_driver_s *dev = NULL;
+ FAR struct net_driver_s *tmpdev;
+ ssize_t arglen;
+ int ret;
ninfo("cmd: %d\n", cmd);
net_lock();
- /* Execute the command */
+ /* Execute the command without ifr_name or lifr_name */
switch (cmd)
{
+ case SIOCGIFCOUNT: /* Get number of devices */
+ {
+ req->ifr_count = netdev_count();
+ ret = OK;
+ }
+ break;
+
#ifdef CONFIG_NET_IPv4
- case SIOCGIFADDR: /* Get IP address */
+ case SIOCGIFCONF: /* Return an interface list (IPv4) */
{
- dev = netdev_ifr_dev(req);
- if (dev)
- {
- ioctl_get_ipv4addr(&req->ifr_addr, dev->d_ipaddr);
- ret = OK;
- }
+ ret = netdev_ipv4_ifconf((FAR struct ifconf *)req);
}
break;
#endif
-#ifdef CONFIG_NET_IPv4
- case SIOCSIFADDR: /* Set IP address */
+#ifdef CONFIG_NET_IPv6
+ case SIOCGLIFCONF: /* Return an interface list (IPv6) */
{
- dev = netdev_ifr_dev(req);
- if (dev)
- {
- ioctl_set_ipv4addr(&dev->d_ipaddr, &req->ifr_addr);
- ret = OK;
- }
+ ret = netdev_ipv6_ifconf((FAR struct lifconf *)req);
}
break;
#endif
-#ifdef CONFIG_NET_IPv4
- case SIOCGIFDSTADDR: /* Get P-to-P address */
+#ifdef CONFIG_NETDEV_IFINDEX
+ case SIOCGIFNAME: /* Get interface name */
{
- dev = netdev_ifr_dev(req);
- if (dev)
+ tmpdev = netdev_findbyindex(req->ifr_ifindex);
+ if (tmpdev != NULL)
{
- ioctl_get_ipv4addr(&req->ifr_dstaddr, dev->d_draddr);
+ strlcpy(req->ifr_name, tmpdev->d_ifname, IFNAMSIZ);
ret = OK;
}
+ else
+ {
+ ret = -ENODEV;
+ }
}
break;
#endif
-
-#ifdef CONFIG_NET_IPv4
- case SIOCSIFDSTADDR: /* Set P-to-P address */
+ default:
{
- dev = netdev_ifr_dev(req);
- if (dev)
+ arglen = net_ioctl_arglen(cmd);
+
+ if (arglen == sizeof(struct ifreq) ||
Review Comment:
why check the size of ifreq or lifreq? it isn't reliable.
##########
net/netdev/netdev_ioctl.c:
##########
@@ -636,235 +636,204 @@ static FAR struct net_driver_s *netdev_ifr_dev(FAR struct ifreq *req)
static int netdev_ifr_ioctl(FAR struct socket *psock, int cmd,
FAR struct ifreq *req)
{
- FAR struct net_driver_s *dev;
- int ret = -EINVAL;
+ FAR struct net_driver_s *dev = NULL;
+ FAR struct net_driver_s *tmpdev;
+ ssize_t arglen;
+ int ret;
ninfo("cmd: %d\n", cmd);
net_lock();
- /* Execute the command */
+ /* Execute the command without ifr_name or lifr_name */
switch (cmd)
{
+ case SIOCGIFCOUNT: /* Get number of devices */
+ {
+ req->ifr_count = netdev_count();
+ ret = OK;
+ }
+ break;
+
#ifdef CONFIG_NET_IPv4
- case SIOCGIFADDR: /* Get IP address */
+ case SIOCGIFCONF: /* Return an interface list (IPv4) */
{
- dev = netdev_ifr_dev(req);
- if (dev)
- {
- ioctl_get_ipv4addr(&req->ifr_addr, dev->d_ipaddr);
- ret = OK;
- }
+ ret = netdev_ipv4_ifconf((FAR struct ifconf *)req);
}
break;
#endif
-#ifdef CONFIG_NET_IPv4
- case SIOCSIFADDR: /* Set IP address */
+#ifdef CONFIG_NET_IPv6
+ case SIOCGLIFCONF: /* Return an interface list (IPv6) */
{
- dev = netdev_ifr_dev(req);
- if (dev)
- {
- ioctl_set_ipv4addr(&dev->d_ipaddr, &req->ifr_addr);
- ret = OK;
- }
+ ret = netdev_ipv6_ifconf((FAR struct lifconf *)req);
}
break;
#endif
-#ifdef CONFIG_NET_IPv4
- case SIOCGIFDSTADDR: /* Get P-to-P address */
+#ifdef CONFIG_NETDEV_IFINDEX
+ case SIOCGIFNAME: /* Get interface name */
{
- dev = netdev_ifr_dev(req);
- if (dev)
+ tmpdev = netdev_findbyindex(req->ifr_ifindex);
Review Comment:
why not use dev directly
--
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
[GitHub] [incubator-nuttx] xiaoxiang781216 merged pull request #7020: net/netdev: simplify handling of netdev ifr ioctl()
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged PR #7020:
URL: https://github.com/apache/incubator-nuttx/pull/7020
--
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
[GitHub] [incubator-nuttx] hartmannathan commented on a diff in pull request #7020: net/netdev: simplify handling of netdev ifr ioctl()
Posted by GitBox <gi...@apache.org>.
hartmannathan commented on code in PR #7020:
URL: https://github.com/apache/incubator-nuttx/pull/7020#discussion_r964248968
##########
net/netdev/netdev_ioctl.c:
##########
@@ -636,235 +636,204 @@ static FAR struct net_driver_s *netdev_ifr_dev(FAR struct ifreq *req)
static int netdev_ifr_ioctl(FAR struct socket *psock, int cmd,
FAR struct ifreq *req)
{
- FAR struct net_driver_s *dev;
- int ret = -EINVAL;
+ FAR struct net_driver_s *dev = NULL;
+ FAR struct net_driver_s *tmpdev;
+ ssize_t arglen;
+ int ret;
ninfo("cmd: %d\n", cmd);
net_lock();
- /* Execute the command */
+ /* Execute the command without ifr_name or lifr_name */
Review Comment:
Maybe change comment to:
`/* Execute commands that do not need ifr_name or lifr_name */`
or something like that?
##########
net/netdev/netdev_ioctl.c:
##########
@@ -636,235 +636,204 @@ static FAR struct net_driver_s *netdev_ifr_dev(FAR struct ifreq *req)
static int netdev_ifr_ioctl(FAR struct socket *psock, int cmd,
FAR struct ifreq *req)
{
- FAR struct net_driver_s *dev;
- int ret = -EINVAL;
+ FAR struct net_driver_s *dev = NULL;
+ FAR struct net_driver_s *tmpdev;
+ ssize_t arglen;
+ int ret;
ninfo("cmd: %d\n", cmd);
net_lock();
- /* Execute the command */
+ /* Execute the command without ifr_name or lifr_name */
switch (cmd)
{
+ case SIOCGIFCOUNT: /* Get number of devices */
+ {
+ req->ifr_count = netdev_count();
+ ret = OK;
+ }
+ break;
+
#ifdef CONFIG_NET_IPv4
- case SIOCGIFADDR: /* Get IP address */
+ case SIOCGIFCONF: /* Return an interface list (IPv4) */
{
- dev = netdev_ifr_dev(req);
- if (dev)
- {
- ioctl_get_ipv4addr(&req->ifr_addr, dev->d_ipaddr);
- ret = OK;
- }
+ ret = netdev_ipv4_ifconf((FAR struct ifconf *)req);
}
break;
#endif
-#ifdef CONFIG_NET_IPv4
- case SIOCSIFADDR: /* Set IP address */
+#ifdef CONFIG_NET_IPv6
+ case SIOCGLIFCONF: /* Return an interface list (IPv6) */
{
- dev = netdev_ifr_dev(req);
- if (dev)
- {
- ioctl_set_ipv4addr(&dev->d_ipaddr, &req->ifr_addr);
- ret = OK;
- }
+ ret = netdev_ipv6_ifconf((FAR struct lifconf *)req);
}
break;
#endif
-#ifdef CONFIG_NET_IPv4
- case SIOCGIFDSTADDR: /* Get P-to-P address */
+#ifdef CONFIG_NETDEV_IFINDEX
+ case SIOCGIFNAME: /* Get interface name */
{
- dev = netdev_ifr_dev(req);
- if (dev)
+ tmpdev = netdev_findbyindex(req->ifr_ifindex);
+ if (tmpdev != NULL)
{
- ioctl_get_ipv4addr(&req->ifr_dstaddr, dev->d_draddr);
+ strlcpy(req->ifr_name, tmpdev->d_ifname, IFNAMSIZ);
ret = OK;
}
+ else
+ {
+ ret = -ENODEV;
+ }
}
break;
#endif
-
-#ifdef CONFIG_NET_IPv4
- case SIOCSIFDSTADDR: /* Set P-to-P address */
+ default:
{
- dev = netdev_ifr_dev(req);
- if (dev)
+ arglen = net_ioctl_arglen(cmd);
+
+ if (arglen == sizeof(struct ifreq) ||
+ arglen == sizeof(struct lifreq))
{
- ioctl_set_ipv4addr(&dev->d_draddr, &req->ifr_dstaddr);
- ret = OK;
+ dev = netdev_ifr_dev(req);
+ ret = (dev == NULL) ? -ENODEV : OK;
+ }
+ else
+ {
+ ret = -ENOTTY;
}
}
break;
-#endif
+ }
+ if (dev == NULL)
+ {
+ return ret;
+ }
+
+ /* Execute the command with ifr_name or lifr_name */
Review Comment:
If changing comment mentioned above, then I suggest to change this one also, for consistency, for example:
`/* Execute commands that need ifr_name or lifr_name */`
##########
net/netdev/netdev_ioctl.c:
##########
@@ -636,235 +636,204 @@ static FAR struct net_driver_s *netdev_ifr_dev(FAR struct ifreq *req)
static int netdev_ifr_ioctl(FAR struct socket *psock, int cmd,
FAR struct ifreq *req)
{
- FAR struct net_driver_s *dev;
- int ret = -EINVAL;
+ FAR struct net_driver_s *dev = NULL;
+ FAR struct net_driver_s *tmpdev;
+ ssize_t arglen;
+ int ret;
ninfo("cmd: %d\n", cmd);
net_lock();
- /* Execute the command */
+ /* Execute the command without ifr_name or lifr_name */
switch (cmd)
{
+ case SIOCGIFCOUNT: /* Get number of devices */
+ {
+ req->ifr_count = netdev_count();
+ ret = OK;
+ }
+ break;
+
#ifdef CONFIG_NET_IPv4
- case SIOCGIFADDR: /* Get IP address */
+ case SIOCGIFCONF: /* Return an interface list (IPv4) */
{
- dev = netdev_ifr_dev(req);
- if (dev)
- {
- ioctl_get_ipv4addr(&req->ifr_addr, dev->d_ipaddr);
- ret = OK;
- }
+ ret = netdev_ipv4_ifconf((FAR struct ifconf *)req);
}
break;
#endif
-#ifdef CONFIG_NET_IPv4
- case SIOCSIFADDR: /* Set IP address */
+#ifdef CONFIG_NET_IPv6
+ case SIOCGLIFCONF: /* Return an interface list (IPv6) */
{
- dev = netdev_ifr_dev(req);
- if (dev)
- {
- ioctl_set_ipv4addr(&dev->d_ipaddr, &req->ifr_addr);
- ret = OK;
- }
+ ret = netdev_ipv6_ifconf((FAR struct lifconf *)req);
}
break;
#endif
-#ifdef CONFIG_NET_IPv4
- case SIOCGIFDSTADDR: /* Get P-to-P address */
+#ifdef CONFIG_NETDEV_IFINDEX
+ case SIOCGIFNAME: /* Get interface name */
{
- dev = netdev_ifr_dev(req);
- if (dev)
+ tmpdev = netdev_findbyindex(req->ifr_ifindex);
Review Comment:
Because originally there was one `switch`, but now there are two.
The first `switch` handles IOCTLs that do not require `dev`, and the `default` gets the `dev`.
Then there is a check if `dev` is `NULL` then we return.
Then, the second `switch` handles all the other IOCTLs.
If use `dev` directly, then we will execute the second `switch` when we shouldn't.
This is a nice refactoring because:
1. Instead of many places getting `dev`, now there is only one. (Reduce code size.)
2. A lot of `#ifdef` rash is reduced by grouping IPv4, IPv6, Ethernet, and 6LOWPAN IOCTLs.
I think it is correct (just by code reading). The diff is hard to read, though, because diff algorithm is grouping things strangely.
--
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
[GitHub] [incubator-nuttx] anchao commented on a diff in pull request #7020: net/netdev: simplify handling of netdev ifr ioctl()
Posted by GitBox <gi...@apache.org>.
anchao commented on code in PR #7020:
URL: https://github.com/apache/incubator-nuttx/pull/7020#discussion_r964337633
##########
net/netdev/netdev_ioctl.c:
##########
@@ -636,235 +636,204 @@ static FAR struct net_driver_s *netdev_ifr_dev(FAR struct ifreq *req)
static int netdev_ifr_ioctl(FAR struct socket *psock, int cmd,
FAR struct ifreq *req)
{
- FAR struct net_driver_s *dev;
- int ret = -EINVAL;
+ FAR struct net_driver_s *dev = NULL;
+ FAR struct net_driver_s *tmpdev;
+ ssize_t arglen;
+ int ret;
ninfo("cmd: %d\n", cmd);
net_lock();
- /* Execute the command */
+ /* Execute the command without ifr_name or lifr_name */
switch (cmd)
{
+ case SIOCGIFCOUNT: /* Get number of devices */
+ {
+ req->ifr_count = netdev_count();
+ ret = OK;
+ }
+ break;
+
#ifdef CONFIG_NET_IPv4
- case SIOCGIFADDR: /* Get IP address */
+ case SIOCGIFCONF: /* Return an interface list (IPv4) */
{
- dev = netdev_ifr_dev(req);
- if (dev)
- {
- ioctl_get_ipv4addr(&req->ifr_addr, dev->d_ipaddr);
- ret = OK;
- }
+ ret = netdev_ipv4_ifconf((FAR struct ifconf *)req);
}
break;
#endif
-#ifdef CONFIG_NET_IPv4
- case SIOCSIFADDR: /* Set IP address */
+#ifdef CONFIG_NET_IPv6
+ case SIOCGLIFCONF: /* Return an interface list (IPv6) */
{
- dev = netdev_ifr_dev(req);
- if (dev)
- {
- ioctl_set_ipv4addr(&dev->d_ipaddr, &req->ifr_addr);
- ret = OK;
- }
+ ret = netdev_ipv6_ifconf((FAR struct lifconf *)req);
}
break;
#endif
-#ifdef CONFIG_NET_IPv4
- case SIOCGIFDSTADDR: /* Get P-to-P address */
+#ifdef CONFIG_NETDEV_IFINDEX
+ case SIOCGIFNAME: /* Get interface name */
{
- dev = netdev_ifr_dev(req);
- if (dev)
+ tmpdev = netdev_findbyindex(req->ifr_ifindex);
+ if (tmpdev != NULL)
{
- ioctl_get_ipv4addr(&req->ifr_dstaddr, dev->d_draddr);
+ strlcpy(req->ifr_name, tmpdev->d_ifname, IFNAMSIZ);
ret = OK;
}
+ else
+ {
+ ret = -ENODEV;
+ }
}
break;
#endif
-
-#ifdef CONFIG_NET_IPv4
- case SIOCSIFDSTADDR: /* Set P-to-P address */
+ default:
{
- dev = netdev_ifr_dev(req);
- if (dev)
+ arglen = net_ioctl_arglen(cmd);
+
+ if (arglen == sizeof(struct ifreq) ||
Review Comment:
I will split the part "struct ifreq" commands out of net_ioctl_arglen() to ensure the consistency of parameter
--
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
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #7020: net/netdev: simplify handling of netdev ifr ioctl()
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #7020:
URL: https://github.com/apache/incubator-nuttx/pull/7020#discussion_r964354825
##########
net/netdev/netdev_ioctl.c:
##########
@@ -636,279 +700,183 @@ static FAR struct net_driver_s *netdev_ifr_dev(FAR struct ifreq *req)
static int netdev_ifr_ioctl(FAR struct socket *psock, int cmd,
FAR struct ifreq *req)
{
- FAR struct net_driver_s *dev;
- int ret = -EINVAL;
+ FAR struct net_driver_s *dev = NULL;
+ int ret = OK;
ninfo("cmd: %d\n", cmd);
net_lock();
- /* Execute the command */
+ /* Execute commands that do not need ifr_name or lifr_name */
switch (cmd)
{
+ case SIOCGIFCOUNT: /* Get number of devices */
+ req->ifr_count = netdev_count();
+ break;
+
#ifdef CONFIG_NET_IPv4
- case SIOCGIFADDR: /* Get IP address */
- {
- dev = netdev_ifr_dev(req);
- if (dev)
- {
- ioctl_get_ipv4addr(&req->ifr_addr, dev->d_ipaddr);
- ret = OK;
- }
- }
+ case SIOCGIFCONF: /* Return an interface list (IPv4) */
+ ret = netdev_ipv4_ifconf((FAR struct ifconf *)req);
break;
#endif
-#ifdef CONFIG_NET_IPv4
- case SIOCSIFADDR: /* Set IP address */
- {
- dev = netdev_ifr_dev(req);
- if (dev)
- {
- ioctl_set_ipv4addr(&dev->d_ipaddr, &req->ifr_addr);
- ret = OK;
- }
- }
+#ifdef CONFIG_NET_IPv6
+ case SIOCGLIFCONF: /* Return an interface list (IPv6) */
+ ret = netdev_ipv6_ifconf((FAR struct lifconf *)req);
break;
#endif
-#ifdef CONFIG_NET_IPv4
- case SIOCGIFDSTADDR: /* Get P-to-P address */
+#ifdef CONFIG_NETDEV_IFINDEX
+ case SIOCGIFNAME: /* Get interface name */
{
- dev = netdev_ifr_dev(req);
- if (dev)
+ FAR struct net_driver_s *tmpdev;
+ tmpdev = netdev_findbyindex(req->ifr_ifindex);
+ if (tmpdev != NULL)
+ {
+ strlcpy(req->ifr_name, tmpdev->d_ifname, IFNAMSIZ);
+ }
+ else
{
- ioctl_get_ipv4addr(&req->ifr_dstaddr, dev->d_draddr);
- ret = OK;
+ ret = -ENODEV;
}
}
break;
#endif
-
-#ifdef CONFIG_NET_IPv4
- case SIOCSIFDSTADDR: /* Set P-to-P address */
+ default:
{
- dev = netdev_ifr_dev(req);
- if (dev)
+ if (net_ioctl_ifreq_arglen(cmd) > 0)
{
- ioctl_set_ipv4addr(&dev->d_draddr, &req->ifr_dstaddr);
- ret = OK;
+ dev = netdev_ifr_dev(req);
+ if (dev == NULL)
+ {
+ ret = -ENODEV;
+ }
+ }
+ else
+ {
+ ret = -ENOTTY;
}
}
break;
-#endif
+ }
+
+ if (dev == NULL)
+ {
+ return ret;
+ }
+
+ /* Execute commands that need ifr_name or lifr_name */
+ switch (cmd)
+ {
#ifdef CONFIG_NET_IPv4
+ case SIOCGIFADDR: /* Get IP address */
+ ioctl_get_ipv4addr(&req->ifr_addr, dev->d_ipaddr);
+ break;
+
+ case SIOCSIFADDR: /* Set IP address */
+ ioctl_set_ipv4addr(&dev->d_ipaddr, &req->ifr_addr);
+ break;
+
+ case SIOCGIFDSTADDR: /* Get P-to-P address */
+ ioctl_get_ipv4addr(&req->ifr_dstaddr, dev->d_draddr);
+ break;
+
+ case SIOCSIFDSTADDR: /* Set P-to-P address */
+ ioctl_set_ipv4addr(&dev->d_draddr, &req->ifr_dstaddr);
+ break;
+
case SIOCGIFBRDADDR: /* Get broadcast IP address */
- {
- dev = netdev_ifr_dev(req);
- if (dev)
- {
- ioctl_get_ipv4broadcast(&req->ifr_broadaddr, dev->d_ipaddr,
- dev->d_netmask);
- ret = OK;
- }
- }
+ ioctl_get_ipv4broadcast(&req->ifr_broadaddr, dev->d_ipaddr,
+ dev->d_netmask);
break;
-#endif
-#ifdef CONFIG_NET_IPv4
case SIOCSIFBRDADDR: /* Set broadcast IP address */
- {
- ret = -ENOSYS;
- }
+ ret = -ENOSYS;
break;
-#endif
-#ifdef CONFIG_NET_IPv4
case SIOCGIFNETMASK: /* Get network mask */
- {
- dev = netdev_ifr_dev(req);
- if (dev)
- {
- ioctl_get_ipv4addr(&req->ifr_addr, dev->d_netmask);
- ret = OK;
- }
- }
+ ioctl_get_ipv4addr(&req->ifr_addr, dev->d_netmask);
break;
-#endif
-#ifdef CONFIG_NET_IPv4
case SIOCSIFNETMASK: /* Set network mask */
- {
- dev = netdev_ifr_dev(req);
- if (dev)
- {
- ioctl_set_ipv4addr(&dev->d_netmask, &req->ifr_addr);
- ret = OK;
- }
- }
+ ioctl_set_ipv4addr(&dev->d_netmask, &req->ifr_addr);
break;
#endif
#ifdef CONFIG_NET_IPv6
case SIOCGLIFADDR: /* Get IP address */
- {
- dev = netdev_ifr_dev(req);
- if (dev)
- {
- FAR struct lifreq *lreq = (FAR struct lifreq *)req;
-
- ioctl_get_ipv6addr(&lreq->lifr_addr, dev->d_ipv6addr);
- ret = OK;
- }
- }
+ FAR struct lifreq *lreq = (FAR struct lifreq *)req;
Review Comment:
need {}
##########
net/netdev/netdev_ioctl.c:
##########
@@ -636,279 +700,183 @@ static FAR struct net_driver_s *netdev_ifr_dev(FAR struct ifreq *req)
static int netdev_ifr_ioctl(FAR struct socket *psock, int cmd,
FAR struct ifreq *req)
{
- FAR struct net_driver_s *dev;
- int ret = -EINVAL;
+ FAR struct net_driver_s *dev = NULL;
+ int ret = OK;
ninfo("cmd: %d\n", cmd);
net_lock();
- /* Execute the command */
+ /* Execute commands that do not need ifr_name or lifr_name */
switch (cmd)
{
+ case SIOCGIFCOUNT: /* Get number of devices */
+ req->ifr_count = netdev_count();
+ break;
+
#ifdef CONFIG_NET_IPv4
- case SIOCGIFADDR: /* Get IP address */
- {
- dev = netdev_ifr_dev(req);
- if (dev)
- {
- ioctl_get_ipv4addr(&req->ifr_addr, dev->d_ipaddr);
- ret = OK;
- }
- }
+ case SIOCGIFCONF: /* Return an interface list (IPv4) */
+ ret = netdev_ipv4_ifconf((FAR struct ifconf *)req);
break;
#endif
-#ifdef CONFIG_NET_IPv4
- case SIOCSIFADDR: /* Set IP address */
- {
- dev = netdev_ifr_dev(req);
- if (dev)
- {
- ioctl_set_ipv4addr(&dev->d_ipaddr, &req->ifr_addr);
- ret = OK;
- }
- }
+#ifdef CONFIG_NET_IPv6
+ case SIOCGLIFCONF: /* Return an interface list (IPv6) */
+ ret = netdev_ipv6_ifconf((FAR struct lifconf *)req);
break;
#endif
-#ifdef CONFIG_NET_IPv4
- case SIOCGIFDSTADDR: /* Get P-to-P address */
+#ifdef CONFIG_NETDEV_IFINDEX
+ case SIOCGIFNAME: /* Get interface name */
{
- dev = netdev_ifr_dev(req);
- if (dev)
+ FAR struct net_driver_s *tmpdev;
+ tmpdev = netdev_findbyindex(req->ifr_ifindex);
+ if (tmpdev != NULL)
+ {
+ strlcpy(req->ifr_name, tmpdev->d_ifname, IFNAMSIZ);
+ }
+ else
{
- ioctl_get_ipv4addr(&req->ifr_dstaddr, dev->d_draddr);
- ret = OK;
+ ret = -ENODEV;
}
}
break;
#endif
-
-#ifdef CONFIG_NET_IPv4
- case SIOCSIFDSTADDR: /* Set P-to-P address */
+ default:
{
- dev = netdev_ifr_dev(req);
- if (dev)
+ if (net_ioctl_ifreq_arglen(cmd) > 0)
{
- ioctl_set_ipv4addr(&dev->d_draddr, &req->ifr_dstaddr);
- ret = OK;
+ dev = netdev_ifr_dev(req);
+ if (dev == NULL)
+ {
+ ret = -ENODEV;
+ }
+ }
+ else
+ {
+ ret = -ENOTTY;
}
}
break;
-#endif
+ }
+
+ if (dev == NULL)
+ {
+ return ret;
+ }
+
+ /* Execute commands that need ifr_name or lifr_name */
+ switch (cmd)
+ {
#ifdef CONFIG_NET_IPv4
+ case SIOCGIFADDR: /* Get IP address */
+ ioctl_get_ipv4addr(&req->ifr_addr, dev->d_ipaddr);
+ break;
+
+ case SIOCSIFADDR: /* Set IP address */
+ ioctl_set_ipv4addr(&dev->d_ipaddr, &req->ifr_addr);
+ break;
+
+ case SIOCGIFDSTADDR: /* Get P-to-P address */
+ ioctl_get_ipv4addr(&req->ifr_dstaddr, dev->d_draddr);
+ break;
+
+ case SIOCSIFDSTADDR: /* Set P-to-P address */
+ ioctl_set_ipv4addr(&dev->d_draddr, &req->ifr_dstaddr);
+ break;
+
case SIOCGIFBRDADDR: /* Get broadcast IP address */
- {
- dev = netdev_ifr_dev(req);
- if (dev)
- {
- ioctl_get_ipv4broadcast(&req->ifr_broadaddr, dev->d_ipaddr,
- dev->d_netmask);
- ret = OK;
- }
- }
+ ioctl_get_ipv4broadcast(&req->ifr_broadaddr, dev->d_ipaddr,
+ dev->d_netmask);
break;
-#endif
-#ifdef CONFIG_NET_IPv4
case SIOCSIFBRDADDR: /* Set broadcast IP address */
- {
- ret = -ENOSYS;
- }
+ ret = -ENOSYS;
break;
-#endif
-#ifdef CONFIG_NET_IPv4
case SIOCGIFNETMASK: /* Get network mask */
- {
- dev = netdev_ifr_dev(req);
- if (dev)
- {
- ioctl_get_ipv4addr(&req->ifr_addr, dev->d_netmask);
- ret = OK;
- }
- }
+ ioctl_get_ipv4addr(&req->ifr_addr, dev->d_netmask);
break;
-#endif
-#ifdef CONFIG_NET_IPv4
case SIOCSIFNETMASK: /* Set network mask */
- {
- dev = netdev_ifr_dev(req);
- if (dev)
- {
- ioctl_set_ipv4addr(&dev->d_netmask, &req->ifr_addr);
- ret = OK;
- }
- }
+ ioctl_set_ipv4addr(&dev->d_netmask, &req->ifr_addr);
break;
#endif
#ifdef CONFIG_NET_IPv6
case SIOCGLIFADDR: /* Get IP address */
- {
- dev = netdev_ifr_dev(req);
- if (dev)
- {
- FAR struct lifreq *lreq = (FAR struct lifreq *)req;
-
- ioctl_get_ipv6addr(&lreq->lifr_addr, dev->d_ipv6addr);
- ret = OK;
- }
- }
+ FAR struct lifreq *lreq = (FAR struct lifreq *)req;
+ ioctl_get_ipv6addr(&lreq->lifr_addr, dev->d_ipv6addr);
break;
-#endif
-#ifdef CONFIG_NET_IPv6
case SIOCSLIFADDR: /* Set IP address */
- {
- dev = netdev_ifr_dev(req);
- if (dev)
- {
- FAR struct lifreq *lreq = (FAR struct lifreq *)req;
-
- ioctl_set_ipv6addr(dev->d_ipv6addr, &lreq->lifr_addr);
- ret = OK;
- }
- }
+ FAR struct lifreq *lreq = (FAR struct lifreq *)req;
Review Comment:
ditto
--
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
[GitHub] [incubator-nuttx] anchao commented on pull request #7020: net/netdev: simplify handling of netdev ifr ioctl()
Posted by GitBox <gi...@apache.org>.
anchao commented on PR #7020:
URL: https://github.com/apache/incubator-nuttx/pull/7020#issuecomment-1240248268
@masayuki2009 ,
sorry for the regression, I uploaded a PR to fix this issue, please review this PR.
https://github.com/apache/incubator-nuttx/pull/7038
--
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
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #7020: net/netdev: simplify handling of netdev ifr ioctl()
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #7020:
URL: https://github.com/apache/incubator-nuttx/pull/7020#discussion_r964468069
##########
net/netdev/netdev_ioctl.c:
##########
@@ -616,6 +616,70 @@ static FAR struct net_driver_s *netdev_ifr_dev(FAR struct ifreq *req)
return NULL;
}
+/****************************************************************************
+ * Name: net_ioctl_ifreq_arglen
+ *
+ * Description:
+ * Calculate the ioctl argument buffer length of ifreq.
+ *
+ * Input Parameters:
+ *
+ * cmd The ioctl command
+ *
+ * Returned Value:
+ * The argument buffer length, or error code.
+ *
+ ****************************************************************************/
+
+static ssize_t net_ioctl_ifreq_arglen(int cmd)
+{
+ switch (cmd)
+ {
+ case SIOCGIFADDR:
+ case SIOCSIFADDR:
+ case SIOCGIFDSTADDR:
+ case SIOCSIFDSTADDR:
+ case SIOCGIFBRDADDR:
+ case SIOCSIFBRDADDR:
+ case SIOCGIFNETMASK:
+ case SIOCSIFNETMASK:
+ case SIOCGIFMTU:
+ case SIOCGIFHWADDR:
+ case SIOCSIFHWADDR:
+ case SIOCDIFADDR:
+ case SIOCGIFCOUNT:
+ case SIOCSIFFLAGS:
+ case SIOCGIFFLAGS:
+ case SIOCMIINOTIFY:
+ case SIOCGMIIPHY:
+ case SIOCGMIIREG:
+ case SIOCSMIIREG:
+ case SIOCGCANBITRATE:
+ case SIOCSCANBITRATE:
+ case SIOCACANEXTFILTER:
+ case SIOCDCANEXTFILTER:
+ case SIOCACANSTDFILTER:
+ case SIOCDCANSTDFILTER:
+ case SIOCGIFNAME:
+ case SIOCGIFINDEX:
+ return sizeof(struct ifreq);
+
+ case SIOCGLIFADDR:
+ case SIOCSLIFADDR:
+ case SIOCGLIFDSTADDR:
+ case SIOCSLIFDSTADDR:
+ case SIOCGLIFBRDADDR:
+ case SIOCSLIFBRDADDR:
+ case SIOCGLIFNETMASK:
+ case SIOCSLIFNETMASK:
+ case SIOCGLIFMTU:
+ case SIOCIFAUTOCONF:
+ return sizeof(struct lifreq);
+ }
Review Comment:
could you please add `default: break;` to make some static code analyzers happy?
--
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
[GitHub] [incubator-nuttx] anchao commented on a diff in pull request #7020: net/netdev: simplify handling of netdev ifr ioctl()
Posted by GitBox <gi...@apache.org>.
anchao commented on code in PR #7020:
URL: https://github.com/apache/incubator-nuttx/pull/7020#discussion_r964336747
##########
net/netdev/netdev_ioctl.c:
##########
@@ -636,235 +636,204 @@ static FAR struct net_driver_s *netdev_ifr_dev(FAR struct ifreq *req)
static int netdev_ifr_ioctl(FAR struct socket *psock, int cmd,
FAR struct ifreq *req)
{
- FAR struct net_driver_s *dev;
- int ret = -EINVAL;
+ FAR struct net_driver_s *dev = NULL;
+ FAR struct net_driver_s *tmpdev;
+ ssize_t arglen;
+ int ret;
ninfo("cmd: %d\n", cmd);
net_lock();
- /* Execute the command */
+ /* Execute the command without ifr_name or lifr_name */
Review Comment:
Thank you, done!
##########
net/netdev/netdev_ioctl.c:
##########
@@ -636,235 +636,204 @@ static FAR struct net_driver_s *netdev_ifr_dev(FAR struct ifreq *req)
static int netdev_ifr_ioctl(FAR struct socket *psock, int cmd,
FAR struct ifreq *req)
{
- FAR struct net_driver_s *dev;
- int ret = -EINVAL;
+ FAR struct net_driver_s *dev = NULL;
+ FAR struct net_driver_s *tmpdev;
+ ssize_t arglen;
+ int ret;
ninfo("cmd: %d\n", cmd);
net_lock();
- /* Execute the command */
+ /* Execute the command without ifr_name or lifr_name */
switch (cmd)
{
+ case SIOCGIFCOUNT: /* Get number of devices */
+ {
+ req->ifr_count = netdev_count();
+ ret = OK;
+ }
+ break;
+
#ifdef CONFIG_NET_IPv4
- case SIOCGIFADDR: /* Get IP address */
+ case SIOCGIFCONF: /* Return an interface list (IPv4) */
{
- dev = netdev_ifr_dev(req);
- if (dev)
- {
- ioctl_get_ipv4addr(&req->ifr_addr, dev->d_ipaddr);
- ret = OK;
- }
+ ret = netdev_ipv4_ifconf((FAR struct ifconf *)req);
}
break;
#endif
-#ifdef CONFIG_NET_IPv4
- case SIOCSIFADDR: /* Set IP address */
+#ifdef CONFIG_NET_IPv6
+ case SIOCGLIFCONF: /* Return an interface list (IPv6) */
{
- dev = netdev_ifr_dev(req);
- if (dev)
- {
- ioctl_set_ipv4addr(&dev->d_ipaddr, &req->ifr_addr);
- ret = OK;
- }
+ ret = netdev_ipv6_ifconf((FAR struct lifconf *)req);
}
break;
#endif
-#ifdef CONFIG_NET_IPv4
- case SIOCGIFDSTADDR: /* Get P-to-P address */
+#ifdef CONFIG_NETDEV_IFINDEX
+ case SIOCGIFNAME: /* Get interface name */
{
- dev = netdev_ifr_dev(req);
- if (dev)
+ tmpdev = netdev_findbyindex(req->ifr_ifindex);
+ if (tmpdev != NULL)
{
- ioctl_get_ipv4addr(&req->ifr_dstaddr, dev->d_draddr);
+ strlcpy(req->ifr_name, tmpdev->d_ifname, IFNAMSIZ);
ret = OK;
}
+ else
+ {
+ ret = -ENODEV;
+ }
}
break;
#endif
-
-#ifdef CONFIG_NET_IPv4
- case SIOCSIFDSTADDR: /* Set P-to-P address */
+ default:
{
- dev = netdev_ifr_dev(req);
- if (dev)
+ arglen = net_ioctl_arglen(cmd);
+
+ if (arglen == sizeof(struct ifreq) ||
+ arglen == sizeof(struct lifreq))
{
- ioctl_set_ipv4addr(&dev->d_draddr, &req->ifr_dstaddr);
- ret = OK;
+ dev = netdev_ifr_dev(req);
+ ret = (dev == NULL) ? -ENODEV : OK;
+ }
+ else
+ {
+ ret = -ENOTTY;
}
}
break;
-#endif
+ }
+ if (dev == NULL)
+ {
+ return ret;
+ }
+
+ /* Execute the command with ifr_name or lifr_name */
Review Comment:
Done
--
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
[GitHub] [incubator-nuttx] hartmannathan commented on a diff in pull request #7020: net/netdev: simplify handling of netdev ifr ioctl()
Posted by GitBox <gi...@apache.org>.
hartmannathan commented on code in PR #7020:
URL: https://github.com/apache/incubator-nuttx/pull/7020#discussion_r964255606
##########
net/netdev/netdev_ioctl.c:
##########
@@ -636,235 +636,204 @@ static FAR struct net_driver_s *netdev_ifr_dev(FAR struct ifreq *req)
static int netdev_ifr_ioctl(FAR struct socket *psock, int cmd,
FAR struct ifreq *req)
{
- FAR struct net_driver_s *dev;
- int ret = -EINVAL;
+ FAR struct net_driver_s *dev = NULL;
+ FAR struct net_driver_s *tmpdev;
+ ssize_t arglen;
+ int ret;
ninfo("cmd: %d\n", cmd);
net_lock();
- /* Execute the command */
+ /* Execute the command without ifr_name or lifr_name */
switch (cmd)
{
+ case SIOCGIFCOUNT: /* Get number of devices */
+ {
+ req->ifr_count = netdev_count();
+ ret = OK;
+ }
+ break;
+
#ifdef CONFIG_NET_IPv4
- case SIOCGIFADDR: /* Get IP address */
+ case SIOCGIFCONF: /* Return an interface list (IPv4) */
{
- dev = netdev_ifr_dev(req);
- if (dev)
- {
- ioctl_get_ipv4addr(&req->ifr_addr, dev->d_ipaddr);
- ret = OK;
- }
+ ret = netdev_ipv4_ifconf((FAR struct ifconf *)req);
}
break;
#endif
-#ifdef CONFIG_NET_IPv4
- case SIOCSIFADDR: /* Set IP address */
+#ifdef CONFIG_NET_IPv6
+ case SIOCGLIFCONF: /* Return an interface list (IPv6) */
{
- dev = netdev_ifr_dev(req);
- if (dev)
- {
- ioctl_set_ipv4addr(&dev->d_ipaddr, &req->ifr_addr);
- ret = OK;
- }
+ ret = netdev_ipv6_ifconf((FAR struct lifconf *)req);
}
break;
#endif
-#ifdef CONFIG_NET_IPv4
- case SIOCGIFDSTADDR: /* Get P-to-P address */
+#ifdef CONFIG_NETDEV_IFINDEX
+ case SIOCGIFNAME: /* Get interface name */
{
- dev = netdev_ifr_dev(req);
- if (dev)
+ tmpdev = netdev_findbyindex(req->ifr_ifindex);
Review Comment:
Maybe should declare `tmpdev` inside the block where it is used? This will also avoid future mistakes of wrongfully using `tmpdev` instead of `dev` in other places.
--
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