Re: [Bugme-new] [Bug 7708] New: unregister_netdev() should return unregister_netdevice() return code

Re: [Bugme-new] [Bug 7708] New: unregister_netdev() should return unregister_netdevice() return code

[Andrew Morton]

On Mon, 18 Dec 2006 14:56:31 -0800
bugme-daemon@bugzilla.kernel.org wrote:

> http://bugzilla.kernel.org/show_bug.cgi?id=7708
> 
>            Summary: unregister_netdev() should return unregister_netdevice()
>                     return code
>     Kernel Version: 2.6.19.1
>             Status: NEW
>           Severity: low
>              Owner: acme@conectiva.com.br
>          Submitter: benjamin.li@qlogic.com
> 
> 
> net/core/dev.c:unregister_netdev() function is a wrapper around
> net/core/dev.c:unregister_netdevice().  The unregister_netdevice() function
> returns a return code while unregister_netdev() currently does not.  For
> completeness, we should pass the return code from unregister_netdevice() all the
> way to the caller.  unregister_netdev() should not swallow the return code.
> 

Certainly there's some truth in that ;)

Is there some reason why you want to test the unregister_netdev() return
value?

Re: [Bugme-new] [Bug 7708] New: unregister_netdev() should return unregister_netdevice() return code

[Stephen Hemminger]

On Mon, 18 Dec 2006 15:21:11 -0800
Andrew Morton <akpm@osdl.org> wrote:

> On Mon, 18 Dec 2006 14:56:31 -0800
> bugme-daemon@bugzilla.kernel.org wrote:
> 
> > http://bugzilla.kernel.org/show_bug.cgi?id=7708
> > 
> >            Summary: unregister_netdev() should return unregister_netdevice()
> >                     return code
> >     Kernel Version: 2.6.19.1
> >             Status: NEW
> >           Severity: low
> >              Owner: acme@conectiva.com.br
> >          Submitter: benjamin.li@qlogic.com
> > 
> > 
> > net/core/dev.c:unregister_netdev() function is a wrapper around
> > net/core/dev.c:unregister_netdevice().  The unregister_netdevice() function
> > returns a return code while unregister_netdev() currently does not.  For
> > completeness, we should pass the return code from unregister_netdevice() all the
> > way to the caller.  unregister_netdev() should not swallow the return code.
> > 
> 
> Certainly there's some truth in that ;)
> 
> Is there some reason why you want to test the unregister_netdev() return
> value?
The only return value is -ENODEV, so I would vote for both just being void

-- 
Stephen Hemminger <shemminger@osdl.org>

Re: [Bugme-new] [Bug 7708] New: unregister_netdev() should return unregister_netdevice() return code

[David Miller]

From: Stephen Hemminger <shemminger@osdl.org>
Date: Mon, 18 Dec 2006 15:50:57 -0800

> On Mon, 18 Dec 2006 15:21:11 -0800
> Andrew Morton <akpm@osdl.org> wrote:
> 
> > > net/core/dev.c:unregister_netdev() function is a wrapper around
> > > net/core/dev.c:unregister_netdevice().  The unregister_netdevice() function
> > > returns a return code while unregister_netdev() currently does not.  For
> > > completeness, we should pass the return code from unregister_netdevice() all the
> > > way to the caller.  unregister_netdev() should not swallow the return code.
> > > 
> > 
> > Certainly there's some truth in that ;)
> > 
> > Is there some reason why you want to test the unregister_netdev() return
> > value?
> The only return value is -ENODEV, so I would vote for both just being void

Me too.

FWIW, I think bug reports like this are a lot of back-and-forth waste
of time.  If it's important enough to someone, let them write a
god-damn patch for something so amazingly trivial.  With the bugzill
entry, you have to respond to it, change it's diapers, etc.

[PATCH] net: unregister_netdevice as void

[Stephen Hemminger]

There was no real useful information from the unregister_netdevice() return
code, the only error occurred in a situation that was a driver bug. So
change it to a void function.

Signed-off-by: Stephen Hemminger <shemminger@osdl.org>
---
 include/linux/netdevice.h |    2 +-
 net/core/dev.c            |   15 ++++++---------
 net/ipv4/ip_gre.c         |    3 ++-
 net/ipv4/ipip.c           |    3 ++-
 net/ipv6/ip6_tunnel.c     |    3 ++-
 net/ipv6/sit.c            |    3 ++-
 6 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 6be767c..e16d66e 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -588,7 +588,7 @@ extern int		dev_open(struct net_device *
 extern int		dev_close(struct net_device *dev);
 extern int		dev_queue_xmit(struct sk_buff *skb);
 extern int		register_netdevice(struct net_device *dev);
-extern int		unregister_netdevice(struct net_device *dev);
+extern void		unregister_netdevice(struct net_device *dev);
 extern void		free_netdev(struct net_device *dev);
 extern void		synchronize_net(void);
 extern int 		register_netdevice_notifier(struct notifier_block *nb);
diff --git a/net/core/dev.c b/net/core/dev.c
index e660cb5..4a8a7fd 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3247,7 +3247,7 @@ void synchronize_net(void) 
  *	unregister_netdev() instead of this.
  */
 
-int unregister_netdevice(struct net_device *dev)
+void unregister_netdevice(struct net_device *dev)
 {
 	struct net_device *d, **dp;
 
@@ -3258,8 +3258,10 @@ int unregister_netdevice(struct net_devi
 	if (dev->reg_state == NETREG_UNINITIALIZED) {
 		printk(KERN_DEBUG "unregister_netdevice: device %s/%p never "
 				  "was registered\n", dev->name, dev);
-		return -ENODEV;
-	}
+		
+		WARN_ON(1);
+		return;
+	}		
 
 	BUG_ON(dev->reg_state != NETREG_REGISTERED);
 
@@ -3280,11 +3282,7 @@ int unregister_netdevice(struct net_devi
 			break;
 		}
 	}
-	if (!d) {
-		printk(KERN_ERR "unregister net_device: '%s' not found\n",
-		       dev->name);
-		return -ENODEV;
-	}
+	BUG_ON(!d);
 
 	dev->reg_state = NETREG_UNREGISTERING;
 
@@ -3316,7 +3314,6 @@ int unregister_netdevice(struct net_devi
 	synchronize_net();
 
 	dev_put(dev);
-	return 0;
 }
 
 /**
diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index 476cb60..51c8350 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -1008,7 +1008,8 @@ ipgre_tunnel_ioctl (struct net_device *d
 				goto done;
 			dev = t->dev;
 		}
-		err = unregister_netdevice(dev);
+		unregister_netdevice(dev);
+		err = 0;
 		break;
 
 	default:
diff --git a/net/ipv4/ipip.c b/net/ipv4/ipip.c
index 9d719d6..da8bbd2 100644
--- a/net/ipv4/ipip.c
+++ b/net/ipv4/ipip.c
@@ -754,7 +754,8 @@ ipip_tunnel_ioctl (struct net_device *de
 				goto done;
 			dev = t->dev;
 		}
-		err = unregister_netdevice(dev);
+		unregister_netdevice(dev);
+		err = 0;
 		break;
 
 	default:
diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
index 8d91834..2b9e3bb 100644
--- a/net/ipv6/ip6_tunnel.c
+++ b/net/ipv6/ip6_tunnel.c
@@ -999,7 +999,8 @@ ip6ip6_tnl_ioctl(struct net_device *dev,
 				break;
 			dev = t->dev;
 		}
-		err = unregister_netdevice(dev);
+		err = 0;
+		unregister_netdevice(dev);
 		break;
 	default:
 		err = -EINVAL;
diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c
index 77b7b09..47cfead 100644
--- a/net/ipv6/sit.c
+++ b/net/ipv6/sit.c
@@ -686,7 +686,8 @@ ipip6_tunnel_ioctl (struct net_device *d
 				goto done;
 			dev = t->dev;
 		}
-		err = unregister_netdevice(dev);
+		unregister_netdevice(dev);
+		err = 0;
 		break;
 
 	default:
-- 
1.4.2.4

Re: [PATCH] net: unregister_netdevice as void

[David Miller]

From: Stephen Hemminger <shemminger@osdl.org>
Date: Mon, 18 Dec 2006 21:05:11 -0800

> There was no real useful information from the unregister_netdevice() return
> code, the only error occurred in a situation that was a driver bug. So
> change it to a void function.
> 
> Signed-off-by: Stephen Hemminger <shemminger@osdl.org>

Applied to net-2.6.21, thanks Stephen.