* [KJ] [PATCH] olympic.c: Checks for return values
@ 2006-09-16 8:40 Hashem Masoud
2006-09-16 9:14 ` Frederik Deweerdt
` (3 more replies)
0 siblings, 4 replies; 5+ messages in thread
From: Hashem Masoud @ 2006-09-16 8:40 UTC (permalink / raw)
To: kernel-janitors
Hello all,
As per the TODO list on your website, register_netdev() should be
audited for its return values.
The following could be an instance of wrong handling of return code.
I didn't compile this as I don't have the proper setup. Added to that, I
am not sure if
the goto is the proper action. Please correct the patch if it is wrong,
it is my first patch!
Signed-off-by: Hashem Masoud < <https://lists.osdl.org/mailman/listinfo/kernel-janitors> masoudh at batelco.com.bh>
---
--- linux-2.6.17.13/drivers/net/tokenring/olympic.c 2006-09-09 06:23:24.000000000 +0300
+++ olympic.c 2006-09-15 22:11:09.099112040 +0300
@@ -265,7 +265,10 @@ static int __devinit olympic_probe(struc
SET_NETDEV_DEV(dev, &pdev->dev);
pci_set_drvdata(pdev,dev) ;
- register_netdev(dev) ;
+
+ if (register_netdev(dev) != 0)
+ goto op_free_dev;
+
printk("Olympic: %s registered as: %s\n",olympic_priv->olympic_card_name,dev->name);
if (olympic_priv->olympic_network_monitor) { /* Must go after register_netdev as we need the device name */
char proc_name[20] ;
--
Hashem Masoud
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [KJ] [PATCH] olympic.c: Checks for return values
2006-09-16 8:40 [KJ] [PATCH] olympic.c: Checks for return values Hashem Masoud
@ 2006-09-16 9:14 ` Frederik Deweerdt
2006-09-16 14:18 ` Hashem Masoud
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Frederik Deweerdt @ 2006-09-16 9:14 UTC (permalink / raw)
To: kernel-janitors
On Sat, Sep 16, 2006 at 11:40:51AM +0300, Hashem Masoud wrote:
> Hello all,
Hi Hashem,
>
> As per the TODO list on your website, register_netdev() should be
> audited for its return values.
> The following could be an instance of wrong handling of return code.
>
> I didn't compile this as I don't have the proper setup. Added to that, I
> am not sure if
> the goto is the proper action. Please correct the patch if it is wrong,
> it is my first patch!
>
If you goto 'op_free_dev', you'll leave the mem io mapped (see
'op_free_iomap').
Regards,
Frederik
>
> Signed-off-by: Hashem Masoud < <https://lists.osdl.org/mailman/listinfo/kernel-janitors> masoudh at batelco.com.bh>
> ---
> --- linux-2.6.17.13/drivers/net/tokenring/olympic.c 2006-09-09 06:23:24.000000000 +0300
> +++ olympic.c 2006-09-15 22:11:09.099112040 +0300
> @@ -265,7 +265,10 @@ static int __devinit olympic_probe(struc
> SET_NETDEV_DEV(dev, &pdev->dev);
>
> pci_set_drvdata(pdev,dev) ;
> - register_netdev(dev) ;
> +
> + if (register_netdev(dev) != 0)
> + goto op_free_dev;
> +
> printk("Olympic: %s registered as: %s\n",olympic_priv->olympic_card_name,dev->name);
> if (olympic_priv->olympic_network_monitor) { /* Must go after register_netdev as we need the device name */
> char proc_name[20] ;
>
>
> --
> Hashem Masoud
>
> _______________________________________________
> Kernel-janitors mailing list
> Kernel-janitors@lists.osdl.org
> https://lists.osdl.org/mailman/listinfo/kernel-janitors
>
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [KJ] [PATCH] olympic.c: Checks for return values
2006-09-16 8:40 [KJ] [PATCH] olympic.c: Checks for return values Hashem Masoud
2006-09-16 9:14 ` Frederik Deweerdt
@ 2006-09-16 14:18 ` Hashem Masoud
2006-09-16 15:46 ` Alexey Dobriyan
2006-09-17 13:21 ` Hashem Masoud
3 siblings, 0 replies; 5+ messages in thread
From: Hashem Masoud @ 2006-09-16 14:18 UTC (permalink / raw)
To: kernel-janitors
Frederik Deweerdt wrote:
>If you goto 'op_free_dev', you'll leave the mem io mapped (see
>'op_free_iomap').
>Regards,
>Frederik
>
>
>
would this be the solution then?
Signed-off-by: Hashem Masoud < masoudh at batelco.com.bh>
---
--- /linux-2.6.17.13/drivers/net/tokenring/olympic.c 2006-09-09
06:23:24.000000000 +0300
+++ olympic.c 2006-09-16 17:12:07.233793920 +0300
@@ -265,7 +265,10 @@ static int __devinit olympic_probe(struc
SET_NETDEV_DEV(dev, &pdev->dev);
pci_set_drvdata(pdev,dev) ;
- register_netdev(dev) ;
+
+ if (register_netdev(dev) != 0)
+ goto op_free_iomap ;
+
printk("Olympic: %s registered as:
%s\n",olympic_priv->olympic_card_name,dev->name);
if (olympic_priv->olympic_network_monitor) { /* Must go after
register_netdev as we need the device name */
char proc_name[20] ;
--
Hashem Masoud
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [KJ] [PATCH] olympic.c: Checks for return values
2006-09-16 8:40 [KJ] [PATCH] olympic.c: Checks for return values Hashem Masoud
2006-09-16 9:14 ` Frederik Deweerdt
2006-09-16 14:18 ` Hashem Masoud
@ 2006-09-16 15:46 ` Alexey Dobriyan
2006-09-17 13:21 ` Hashem Masoud
3 siblings, 0 replies; 5+ messages in thread
From: Alexey Dobriyan @ 2006-09-16 15:46 UTC (permalink / raw)
To: kernel-janitors
On Sat, Sep 16, 2006 at 05:18:47PM +0300, Hashem Masoud wrote:
> --- /linux-2.6.17.13/drivers/net/tokenring/olympic.c
> +++ olympic.c
> @@ -265,7 +265,10 @@ static int __devinit olympic_probe(struc
> SET_NETDEV_DEV(dev, &pdev->dev);
>
> pci_set_drvdata(pdev,dev) ;
> - register_netdev(dev) ;
> +
> + if (register_netdev(dev) != 0)
> + goto op_free_iomap ;
> +
> printk("Olympic: %s registered as:
> %s\n",olympic_priv->olympic_card_name,dev->name);
> if (olympic_priv->olympic_network_monitor) { /* Must go after
> register_netdev as we need the device name */
> char proc_name[20] ;
That's almost it. However if you follow goto you'll notice that zero
will be returned which is not a proper indication of error.
Ergo, I need you to propagate error from register_netdev(),
read http://mbligh.org/linuxdocs/Email/Clients/Thunderbird , then send
proper inline patch with proper Signed-off-by line (without "at", normal
email). Also, you need to buy Thunderbird voodoo doll.
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [KJ] [PATCH] olympic.c: Checks for return values
2006-09-16 8:40 [KJ] [PATCH] olympic.c: Checks for return values Hashem Masoud
` (2 preceding siblings ...)
2006-09-16 15:46 ` Alexey Dobriyan
@ 2006-09-17 13:21 ` Hashem Masoud
3 siblings, 0 replies; 5+ messages in thread
From: Hashem Masoud @ 2006-09-17 13:21 UTC (permalink / raw)
To: kernel-janitors
Alexey Dobriyan wrote:
> That's almost it. However if you follow goto you'll notice that zero
> will be returned which is not a proper indication of error.
>
> Ergo, I need you to propagate error from register_netdev(),
> read http://mbligh.org/linuxdocs/Email/Clients/Thunderbird , then send
> proper inline patch with proper Signed-off-by line (without "at", normal
> email). Also, you need to buy Thunderbird voodoo doll.
>
>
>
Thanks, this is the newest patch:
Signed-off-by: Hashem Masoud <masoudh@batelco.com.bh>
---
--- linux-2.6.17.13/drivers/net/tokenring/olympic.c 2006-09-09 06:23:24.000000000 +0300
+++ olympic.c 2006-09-17 15:47:18.293705248 +0300
@@ -265,7 +265,11 @@ static int __devinit olympic_probe(struc
SET_NETDEV_DEV(dev, &pdev->dev);
pci_set_drvdata(pdev,dev) ;
- register_netdev(dev) ;
+
+ if ((i = register_netdev(dev))) {
+ goto op_free_iomap;
+ }
+
printk("Olympic: %s registered as: %s\n",olympic_priv->olympic_card_name,dev->name);
if (olympic_priv->olympic_network_monitor) { /* Must go after register_netdev as we need the device name */
char proc_name[20] ;
I got the Thunderbird voodoo doll! But mutt is just too hard ;-)
--
Hashem Masoud
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2006-09-17 13:21 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-09-16 8:40 [KJ] [PATCH] olympic.c: Checks for return values Hashem Masoud
2006-09-16 9:14 ` Frederik Deweerdt
2006-09-16 14:18 ` Hashem Masoud
2006-09-16 15:46 ` Alexey Dobriyan
2006-09-17 13:21 ` Hashem Masoud
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.