* Re: [KJ] bonding/bond_main.c: check return value of
2006-02-02 13:37 [KJ] bonding/bond_main.c: check return value of walter harms
@ 2006-02-20 21:09 ` Alexey Dobriyan
2006-02-25 11:53 ` walter harms
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Alexey Dobriyan @ 2006-02-20 21:09 UTC (permalink / raw)
To: kernel-janitors
[-- 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
^ permalink raw reply [flat|nested] 7+ messages in thread* [KJ] bonding/bond_main.c: check return value of
2006-02-02 13:37 [KJ] bonding/bond_main.c: check return value of walter harms
2006-02-20 21:09 ` Alexey Dobriyan
@ 2006-02-25 11:53 ` walter harms
2006-02-25 12:13 ` Jesper Juhl
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: walter harms @ 2006-02-25 11:53 UTC (permalink / raw)
To: kernel-janitors
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
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [KJ] bonding/bond_main.c: check return value of
2006-02-02 13:37 [KJ] bonding/bond_main.c: check return value of walter harms
2006-02-20 21:09 ` Alexey Dobriyan
2006-02-25 11:53 ` walter harms
@ 2006-02-25 12:13 ` Jesper Juhl
2006-02-25 12:31 ` Tim Cooijmans
` (2 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Jesper Juhl @ 2006-02-25 12:13 UTC (permalink / raw)
To: kernel-janitors
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
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [KJ] bonding/bond_main.c: check return value of
2006-02-02 13:37 [KJ] bonding/bond_main.c: check return value of walter harms
` (2 preceding siblings ...)
2006-02-25 12:13 ` Jesper Juhl
@ 2006-02-25 12:31 ` Tim Cooijmans
2006-02-25 12:40 ` walter harms
2006-02-25 12:52 ` Jesper Juhl
5 siblings, 0 replies; 7+ messages in thread
From: Tim Cooijmans @ 2006-02-25 12:31 UTC (permalink / raw)
To: kernel-janitors
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
^ permalink raw reply [flat|nested] 7+ messages in thread* [KJ] bonding/bond_main.c: check return value of
2006-02-02 13:37 [KJ] bonding/bond_main.c: check return value of walter harms
` (3 preceding siblings ...)
2006-02-25 12:31 ` Tim Cooijmans
@ 2006-02-25 12:40 ` walter harms
2006-02-25 12:52 ` Jesper Juhl
5 siblings, 0 replies; 7+ messages in thread
From: walter harms @ 2006-02-25 12:40 UTC (permalink / raw)
To: kernel-janitors
>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
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [KJ] bonding/bond_main.c: check return value of
2006-02-02 13:37 [KJ] bonding/bond_main.c: check return value of walter harms
` (4 preceding siblings ...)
2006-02-25 12:40 ` walter harms
@ 2006-02-25 12:52 ` Jesper Juhl
5 siblings, 0 replies; 7+ messages in thread
From: Jesper Juhl @ 2006-02-25 12:52 UTC (permalink / raw)
To: kernel-janitors
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
^ permalink raw reply [flat|nested] 7+ messages in thread