NetBSD Problem Report #38976

From www@NetBSD.org  Wed Jun 18 02:49:10 2008
Return-Path: <www@NetBSD.org>
Received: from mail.netbsd.org (mail.netbsd.org [204.152.190.11])
	by narn.NetBSD.org (Postfix) with ESMTP id 38FC763B8E3
	for <gnats-bugs@gnats.netbsd.org>; Wed, 18 Jun 2008 02:49:10 +0000 (UTC)
Message-Id: <20080618024909.DA43C63B842@narn.NetBSD.org>
Date: Wed, 18 Jun 2008 02:49:09 +0000 (UTC)
From: g.mcgarry@ieee.org
Reply-To: g.mcgarry@ieee.org
To: gnats-bugs@NetBSD.org
Subject: src/sys/net/if_ethersubr.c rev 1.163 fubar
X-Send-Pr-Version: www-1.0

>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:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Wed Jun 18 02:50:00 +0000 2008
>Closed-Date:    Fri Sep 05 22:11:29 +0000 2008
>Last-Modified:  Fri Sep 05 22:11:29 +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.


>Release-Note:

>Audit-Trail:
From: Gregory McGarry <gmcgarry@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/38976 CVS commit: src/sys/net
Date: Wed, 23 Jul 2008 05:41:47 +0000 (UTC)

 Module Name:	src
 Committed By:	gmcgarry
 Date:		Wed Jul 23 05:41:47 UTC 2008

 Modified Files:
 	src/sys/net: if_ethersubr.c

 Log Message:
 Back out rev 1.163 which broke the logic for SIOCSIFFLAGS.  PR#38976.


 To generate a diff of this commit:
 cvs rdiff -r1.167 -r1.168 src/sys/net/if_ethersubr.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: open->closed
State-Changed-By: gmcgarry@NetBSD.org
State-Changed-When: Fri, 05 Sep 2008 22:11:29 +0000
State-Changed-Why:
patch applied.


>Unformatted:

Home	PR Database Search