[KJ] bonding/bond_main.c: check return value of

[KJ] bonding/bond_main.c: check return value of

[walter harms]

[-- Attachment #1: Type: text/plain, Size: 679 bytes --]


check the return value of register_netdevice_notifier

also as attachment if something mangels the layout.
compile tested
Signed-off-by: walter harms <wharms@bfs.de>

--- linux/drivers/net/bonding/bond_main.c.bak	2006-02-01 
22:13:29.000000000 +0100
+++ linux/drivers/net/bonding/bond_main.c	2006-02-01 22:41:31.000000000 
+0100
@@ -4893,7 +4893,14 @@
  	}

  	rtnl_unlock();
-	register_netdevice_notifier(&bond_netdev_notifier);
+
+	res = register_netdevice_notifier(&bond_netdev_notifier);
+
+	if (res < 0) {
+		printk( KERN_WARNING DRV_NAME" register_netdevice_notifier failed\n");
+		goto out_err;
+	}
+
  	register_inetaddr_notifier(&bond_inetaddr_notifier);

  	return 0;

[-- Attachment #2: bond_main.c.diff --]
[-- Type: text/x-patch, Size: 507 bytes --]

--- linux/drivers/net/bonding/bond_main.c.bak	2006-02-01 22:13:29.000000000 +0100
+++ linux/drivers/net/bonding/bond_main.c	2006-02-01 22:41:31.000000000 +0100
@@ -4893,7 +4893,14 @@
 	}
 
 	rtnl_unlock();
-	register_netdevice_notifier(&bond_netdev_notifier);
+
+	res = register_netdevice_notifier(&bond_netdev_notifier);
+
+	if (res < 0) {
+		printk( KERN_WARNING DRV_NAME" register_netdevice_notifier failed\n");
+		goto out_err;
+	}
+
 	register_inetaddr_notifier(&bond_inetaddr_notifier);
 
 	return 0;

[-- Attachment #3: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] bonding/bond_main.c: check return value of

[Alexey Dobriyan]

[-- Attachment #1: Type: text/plain, Size: 624 bytes --]

On Thu, Feb 02, 2006 at 02:37:51PM +0100, walter harms wrote:
> --- linux/drivers/net/bonding/bond_main.c.bak	2006-02-01 22:13:29.000000000 +0100
> +++ linux/drivers/net/bonding/bond_main.c	2006-02-01 22:41:31.000000000 +0100
> @@ -4893,7 +4893,14 @@
>  	}
>
>  	rtnl_unlock();
> -	register_netdevice_notifier(&bond_netdev_notifier);
> +
> +	res = register_netdevice_notifier(&bond_netdev_notifier);
> +
> +	if (res < 0) {
> +		printk( KERN_WARNING DRV_NAME" register_netdevice_notifier failed\n");

This one is OK, except I don't like empty lines introduced. KERN_WARNING
should KERN_ERR really.

> +		goto out_err;
> +	}


[-- Attachment #2: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

[KJ] bonding/bond_main.c: check return value of

[walter harms]

hi list,
my next improved patch. please note
* struct net_device *bond_dev=NULL; to prevend gcc from complaining
* max_bonds is atleast 1 so i should be initialised always
* better error handling (freeing ALL resources in case of error now)
* KERN_ERR instead of KERN_WARNING

The driver does no unregister_netdevice() i did not understand why so i 
left it in this state. The two labels are not a mistake they simply 
illustrate that this is an error exit for two different errors.

alexey, if you realy do not like it, simple remove them.

re,
  walter


Signed-off-by: walter harms <wharms@bfs.de>

--- linux-2.6.15.4/drivers/net/bonding/bond_main.c.bak	2006-02-10 
08:22:48.000000000 +0100
+++ linux-2.6.15.4/drivers/net/bonding/bond_main.c	2006-02-25 
12:36:25.000000000 +0100
@@ -4884,6 +4884,7 @@
  static int __init bonding_init(void)
  {
  	struct bond_params params;
+	struct net_device *bond_dev=NULL;
  	int i;
  	int res;

@@ -4901,7 +4902,7 @@
  #endif

  	for (i = 0; i < max_bonds; i++) {
-		struct net_device *bond_dev;
+

  		bond_dev = alloc_netdev(sizeof(struct bonding), "", ether_setup);
  		if (!bond_dev) {
@@ -4929,18 +4930,29 @@

  		res = register_netdevice(bond_dev);
  		if (res < 0) {
-			bond_deinit(bond_dev);
-			free_netdev(bond_dev);
-			goto out_err;
+			goto out_no_netdevice;
  		}
  	}

  	rtnl_unlock();
-	register_netdevice_notifier(&bond_netdev_notifier);
+	res = register_netdevice_notifier(&bond_netdev_notifier);
+	if (res < 0) {
+		printk( KERN_ERR DRV_NAME" register_netdevice_notifier failed\n");
+		
+		goto out_no_nd_notifier;
+	}
+
+
  	register_inetaddr_notifier(&bond_inetaddr_notifier);

  	return 0;

+out_no_nd_notifier:
+
+out_no_netdevice:
+	bond_deinit(bond_dev);
+	free_netdev(bond_dev);
+
  out_err:
  	/*
  	 * rtnl_unlock() will run netdev_run_todo(), putting the
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] bonding/bond_main.c: check return value of

[Jesper Juhl]

On 2/25/06, walter harms <wharms@bfs.de> wrote:
> hi list,
> my next improved patch. please note
> * struct net_device *bond_dev=NULL; to prevend gcc from complaining
> * max_bonds is atleast 1 so i should be initialised always
> * better error handling (freeing ALL resources in case of error now)
> * KERN_ERR instead of KERN_WARNING
>
> The driver does no unregister_netdevice() i did not understand why so i
> left it in this state. The two labels are not a mistake they simply
> illustrate that this is an error exit for two different errors.
>
> alexey, if you realy do not like it, simple remove them.
>
> re,
>   walter
>
>
> Signed-off-by: walter harms <wharms@bfs.de>
>
> --- linux-2.6.15.4/drivers/net/bonding/bond_main.c.bak  2006-02-10
> 08:22:48.000000000 +0100
> +++ linux-2.6.15.4/drivers/net/bonding/bond_main.c      2006-02-25
> 12:36:25.000000000 +0100
> @@ -4884,6 +4884,7 @@
>   static int __init bonding_init(void)
>   {
>         struct bond_params params;
> +       struct net_device *bond_dev=NULL;

Please observe the kernel coding style, there should be spaces around
the '=' here:

       struct net_device *bond_dev = NULL;


>         int i;
>         int res;
>
> @@ -4901,7 +4902,7 @@
>   #endif
>
>         for (i = 0; i < max_bonds; i++) {
> -               struct net_device *bond_dev;
> +
>

I can see the point in moving the declaration, but why insert a blank
line?  if anything I'd say you should be removing the blank line
that's already there, like so :

 -               struct net_device *bond_dev;
 -


>                 bond_dev = alloc_netdev(sizeof(struct bonding), "", ether_setup);
>                 if (!bond_dev) {
> @@ -4929,18 +4930,29 @@
>
>                 res = register_netdevice(bond_dev);
>                 if (res < 0) {
> -                       bond_deinit(bond_dev);
> -                       free_netdev(bond_dev);
> -                       goto out_err;
> +                       goto out_no_netdevice;
>                 }
>         }
>
>         rtnl_unlock();
> -       register_netdevice_notifier(&bond_netdev_notifier);
> +       res = register_netdevice_notifier(&bond_netdev_notifier);
> +       if (res < 0) {
> +               printk( KERN_ERR DRV_NAME" register_netdevice_notifier failed\n");
> +
> +               goto out_no_nd_notifier;
> +       }
> +
> +
>         register_inetaddr_notifier(&bond_inetaddr_notifier);
>
>         return 0;
>
> +out_no_nd_notifier:
> +
> +out_no_netdevice:

personally I'd just collapse the two labels into a single label.


> +       bond_deinit(bond_dev);
> +       free_netdev(bond_dev);
> +
>   out_err:
>         /*
>          * rtnl_unlock() will run netdev_run_todo(), putting the


--
Jesper Juhl <jesper.juhl@gmail.com>
Don't top-post  http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please      http://www.expita.com/nomime.html

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] bonding/bond_main.c: check return value of

[Tim Cooijmans]

On Saturday 25 February 2006 12:53, walter harms wrote:
> +	res = register_netdevice_notifier(&bond_netdev_notifier);
> +	if (res < 0) {
> +		printk( KERN_ERR DRV_NAME" register_netdevice_notifier failed\n");
> +
> +		goto out_no_nd_notifier;
> +	}
> +
> +
>   	register_inetaddr_notifier(&bond_inetaddr_notifier);
I don't think there's a reason for that double newline.

Tim
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

[KJ] bonding/bond_main.c: check return value of

[walter harms]

 >hi list,
 > my next improved patch. please note
 > * struct net_device *bond_dev=NULL; to prevend gcc from complaining
 > * max_bonds is atleast 1 so i should be initialised always
 > * better error handling (freeing ALL resources in case of error now)
 > * KERN_ERR instead of KERN_WARNING
* less lines more space :)

 >
 > The driver does no unregister_netdevice() i did not understand why so i
 > left it in this state. The two labels are not a mistake they simply
 > illustrate that this is an error exit for two different errors.
 > alexey, if you realy do not like it, simple remove them.
 >
 > re,
 >   walter
ps:
	i admit i did not test-compile it since it is only a fmt issue.


Signed-off-by: walter harms <wharms@bfs.de>

--- linux-2.6.15.4/drivers/net/bonding/bond_main.c.bak	2006-02-10 
08:22:48.000000000 +0100
+++ linux-2.6.15.4/drivers/net/bonding/bond_main.c	2006-02-25 
13:36:10.000000000 +0100
@@ -4884,6 +4884,7 @@
  static int __init bonding_init(void)
  {
  	struct bond_params params;
+	struct net_device *bond_dev = NULL;
  	int i;
  	int res;

@@ -4901,8 +4902,6 @@
  #endif

  	for (i = 0; i < max_bonds; i++) {
-		struct net_device *bond_dev;
-
  		bond_dev = alloc_netdev(sizeof(struct bonding), "", ether_setup);
  		if (!bond_dev) {
  			res = -ENOMEM;
@@ -4929,18 +4928,29 @@

  		res = register_netdevice(bond_dev);
  		if (res < 0) {
-			bond_deinit(bond_dev);
-			free_netdev(bond_dev);
-			goto out_err;
+			goto out_no_netdevice;
  		}
  	}

  	rtnl_unlock();
-	register_netdevice_notifier(&bond_netdev_notifier);
+	res = register_netdevice_notifier(&bond_netdev_notifier);
+	if (res < 0) {
+		printk( KERN_ERR DRV_NAME" register_netdevice_notifier failed\n");
+		
+		goto out_no_nd_notifier;
+	}
+
+
  	register_inetaddr_notifier(&bond_inetaddr_notifier);

  	return 0;

+out_no_nd_notifier:
+
+out_no_netdevice:
+	bond_deinit(bond_dev);
+	free_netdev(bond_dev);
+
  out_err:
  	/*
  	 * rtnl_unlock() will run netdev_run_todo(), putting the
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] bonding/bond_main.c: check return value of

[Jesper Juhl]

On 2/25/06, Tim Cooijmans <tim@aapopfiets.nl> wrote:
> On Saturday 25 February 2006 12:53, walter harms wrote:
> > +     res = register_netdevice_notifier(&bond_netdev_notifier);
> > +     if (res < 0) {
> > +             printk( KERN_ERR DRV_NAME" register_netdevice_notifier failed\n");
> > +
> > +             goto out_no_nd_notifier;
> > +     }
> > +
> > +
> >       register_inetaddr_notifier(&bond_inetaddr_notifier);
> I don't think there's a reason for that double newline.
>
I'd agree, and IMHO the one above the goto should go as well.

--
Jesper Juhl <jesper.juhl@gmail.com>
Don't top-post  http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please      http://www.expita.com/nomime.html

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors