kern/38976: src/sys/net/if_ethersubr.c rev 1.163 fubar

[g.mcgarry%ieee.org@localhost]

>Number:         38976
>Category:       kern
>Synopsis:       src/sys/net/if_ethersubr.c rev 1.163 fubar
>Confidential:   no
>Severity:       critical
>Priority:       high
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Wed Jun 18 02:50:00 +0000 2008
>Originator:     Gregory McGarry
>Release:        -current
>Organization:
>Environment:
>Description:
Revision 1.163 to src/sys/net/if_ethersubr.c is fubar; there is code without a 
case label for SIOCSIFFLAGS.

Wed Mar 12 19:22:24 2008 CET (3 months ago) by dyoung

Make some cosmetic changes:

        Use fewer 'error = ...; break;' statements and more 'return
        ...;'

        Make the SIOCSIFFLAGS case more clear by using a switch
        statement instead of an if-else if-else chain.

        Shorten a staircase, and remove two unnecessary curly
        braces.

Here's is the change:

--- src/sys/net/if_ethersubr.c  2008/02/20 17:05:53     1.162
+++ src/sys/net/if_ethersubr.c  2008/03/12 18:22:24     1.163
@@ -1,4 +1,4 @@
-/*     $NetBSD: if_ethersubr.c,v 1.161 2008/02/07 01:22:00 dyoung Exp $        
*/
+/*     $NetBSD: if_ethersubr.c,v 1.162 2008/02/20 17:05:53 matt Exp $  */
 
 /*
  * Copyright (C) 1995, 1996, 1997, and 1998 WIDE Project.
@@ -61,7 +61,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_ethersubr.c,v 1.161 2008/02/07 01:22:00 dyoung 
Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_ethersubr.c,v 1.162 2008/02/20 17:05:53 matt 
Exp $");
 
 #include "opt_inet.h"
 #include "opt_atalk.h"
@@ -1444,7 +1444,7 @@ ether_ioctl(struct ifnet *ifp, u_long cm
        struct ethercom *ec = (void *) ifp;
        struct ifreq *ifr = (struct ifreq *)data;
        struct ifaddr *ifa = (struct ifaddr *)data;
-       int error = 0;
+       int error;
 
        switch (cmd) {
        case SIOCSIFADDR:
@@ -1455,36 +1455,33 @@ ether_ioctl(struct ifnet *ifp, u_long cm
                        const struct sockaddr_dl *sdl = satocsdl(ifa->ifa_addr);
 
                        if (sdl->sdl_type != IFT_ETHER ||
-                           sdl->sdl_alen != ifp->if_addrlen) {
-                               error = EINVAL;
-                               break;
-                       }
+                           sdl->sdl_alen != ifp->if_addrlen)
+                               return EINVAL;
 
                        if_set_sadl(ifp, CLLADDR(sdl), ifp->if_addrlen);
 
                        /* Set new address. */
-                       error = (*ifp->if_init)(ifp);
-                       break;
+                       return (*ifp->if_init)(ifp);
                    }
 #ifdef INET
                case AF_INET:
                        if ((ifp->if_flags & IFF_RUNNING) == 0 &&
                            (error = (*ifp->if_init)(ifp)) != 0)
-                               break;
+                               return error;
                        arp_ifinit(ifp, ifa);
                        break;
 #endif /* INET */
                default:
                        if ((ifp->if_flags & IFF_RUNNING) == 0)
-                               error = (*ifp->if_init)(ifp);
+                               return (*ifp->if_init)(ifp);
                        break;
                }
-               break;
+               return 0;
 
        case SIOCGIFADDR:
                memcpy(((struct sockaddr *)&ifr->ifr_data)->sa_data,
                    CLLADDR(ifp->if_sadl), ETHER_ADDR_LEN);
-               break;
+               return 0;
 
        case SIOCSIFMTU:
            {
@@ -1496,60 +1493,53 @@ ether_ioctl(struct ifnet *ifp, u_long cm
                        maxmtu = ETHERMTU;
 
                if (ifr->ifr_mtu < ETHERMIN || ifr->ifr_mtu > maxmtu)
-                       error = EINVAL;
-               else if ((error = ifioctl_common(ifp, cmd, data)) == ENETRESET){
+                       return EINVAL;
+               else if ((error = ifioctl_common(ifp, cmd, data)) != ENETRESET)
+                       return error;
+               else if (ifp->if_flags & IFF_UP) {
                        /* Make sure the device notices the MTU change. */
-                       if (ifp->if_flags & IFF_UP)
-                               error = (*ifp->if_init)(ifp);
-                       else
-                               error = 0;
-               }
-               break;
+                       return (*ifp->if_init)(ifp);
+               } else
+                       return 0;
            }
 
        case SIOCSIFFLAGS:
-               if ((ifp->if_flags & (IFF_UP|IFF_RUNNING)) == IFF_RUNNING) {
+               switch (ifp->if_flags & (IFF_UP|IFF_RUNNING)) {
                        /*
                         * If interface is marked down and it is running,
                         * then stop and disable it.
                         */
                        (*ifp->if_stop)(ifp, 1);
-               } else if ((ifp->if_flags & (IFF_UP|IFF_RUNNING)) == IFF_UP) {
+                       break;
+               case IFF_UP:
                        /*
                         * If interface is marked up and it is stopped, then
                         * start it.
                         */
-                       error = (*ifp->if_init)(ifp);
-               } else if ((ifp->if_flags & IFF_UP) != 0) {
+                       return (*ifp->if_init)(ifp);
+               case IFF_UP|IFF_RUNNING:
                        /*
                         * Reset the interface to pick up changes in any other
                         * flags that affect the hardware state.
                         */
-                       error = (*ifp->if_init)(ifp);
+                       return (*ifp->if_init)(ifp);
+               case 0:
+                       break;
                }
-               break;
-
+               return 0;
        case SIOCADDMULTI:
-               error = ether_addmulti(ifreq_getaddr(cmd, ifr), ec);
-               break;
-
+               return ether_addmulti(ifreq_getaddr(cmd, ifr), ec);
        case SIOCDELMULTI:
-               error = ether_delmulti(ifreq_getaddr(cmd, ifr), ec);
-               break;
+               return ether_delmulti(ifreq_getaddr(cmd, ifr), ec);
        case SIOCSIFMEDIA:
        case SIOCGIFMEDIA:
                if (ec->ec_mii == NULL)
-                       error = ENOTTY;
-               else
-                       error = ifmedia_ioctl(ifp, ifr, &ec->ec_mii->mii_media,
-                           cmd);
-               break;
+                       return ENOTTY;
+               return ifmedia_ioctl(ifp, ifr, &ec->ec_mii->mii_media, cmd);
        case SIOCSIFCAP:
                return ifioctl_common(ifp, cmd, data);
        default:
-               error = ENOTTY;
-               break;
+               return ENOTTY;
        }
-
-       return (error);
+       return 0;
 }

>How-To-Repeat:

>Fix:
Revert patch, since it contributes no functional change.