* [KJ][Patch] check return value of request_region in matroxfb_base.c
@ 2006-02-27 8:40 Darren Jenkins\
2006-03-01 7:12 ` [KJ][Patch] check return value of request region in ali5455.c Darren Jenkins\
` (16 more replies)
0 siblings, 17 replies; 18+ messages in thread
From: Darren Jenkins\ @ 2006-02-27 8:40 UTC (permalink / raw)
To: kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 829 bytes --]
G'day list,
matroxfb_base.c calles 'request_region' @ 1737 without checking the
return value.
The patch below adds a check for this and plugs into the existing error
path.
Signed-off-by: Darren Jenkins <darrenrjenkins@gmail.com>
--- linux-2.6.16-rc4/drivers/video/matrox/matroxfb_base.c.orig 2006-02-24 23:44:10.000000000 +1100
+++ linux-2.6.16-rc4/drivers/video/matrox/matroxfb_base.c 2006-02-27 19:26:26.000000000 +1100
@@ -1734,7 +1734,11 @@ static int initMatrox2(WPMINFO struct bo
#endif /* CONFIG_MTRR */
if (!ACCESS_FBINFO(devflags.novga))
- request_region(0x3C0, 32, "matrox");
+ if (!request_region(0x3C0, 32, "matrox")) {
+ printk(KERN_ERR "matroxfb: failed to request_region");
+ err = -EBUSY;
+ goto failVideoIO;
+ }
matroxfb_g450_connect(PMINFO2);
ACCESS_FBINFO(hw_switch->reset(PMINFO2));
[-- 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] 18+ messages in thread
* [KJ][Patch] check return value of request region in ali5455.c
2006-02-27 8:40 [KJ][Patch] check return value of request_region in matroxfb_base.c Darren Jenkins\
@ 2006-03-01 7:12 ` Darren Jenkins\
2006-03-03 6:31 ` [KJ][Patch] check return value of request_region in Petr Vandrovec
` (15 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: Darren Jenkins\ @ 2006-03-01 7:12 UTC (permalink / raw)
To: kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 960 bytes --]
G'day list,
ali5455.c calls request_region without checking it's return value
@ line 3460.
The patch below simply checks the return value and copies local error
style.
Signed-off-by: Darren Jenkins <darrenrjenkins@gmail.com>
--- linux-2.6.16-rc5/sound/oss/ali5455.c.orig 2006-03-01 17:52:40.000000000 +1100
+++ linux-2.6.16-rc5/sound/oss/ali5455.c 2006-03-01 17:58:35.000000000 +1100
@@ -3457,7 +3457,13 @@ static int __devinit ali_probe(struct pc
card->channel[4].port = 0xb0;
card->channel[4].num = 4;
/* claim our iospace and irq */
- request_region(card->iobase, 256, card_names[pci_id->driver_data]);
+ if (!request_region(card->iobase, 256,
+ card_names[pci_id->driver_data])) {
+ printk(KERN_ERR "ali_audio: request_region failed");
+ kfree(card);
+ return -EBUSY
+ }
+
if (request_irq(card->irq, &ali_interrupt, SA_SHIRQ,
card_names[pci_id->driver_data], card)) {
printk(KERN_ERR "ali_audio: unable to allocate irq %d\n",
[-- 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] 18+ messages in thread
* Re: [KJ][Patch] check return value of request_region in
2006-02-27 8:40 [KJ][Patch] check return value of request_region in matroxfb_base.c Darren Jenkins\
2006-03-01 7:12 ` [KJ][Patch] check return value of request region in ali5455.c Darren Jenkins\
@ 2006-03-03 6:31 ` Petr Vandrovec
2006-03-11 13:48 ` [KJ][Patch] check return value of request_irq in amiserial.c Darren Jenkins\
` (14 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: Petr Vandrovec @ 2006-03-03 6:31 UTC (permalink / raw)
To: kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 1417 bytes --]
Darren Jenkins\ wrote:
> G'day list,
>
> matroxfb_base.c calles 'request_region' @ 1737 without checking the
> return value.
>
> The patch below adds a check for this and plugs into the existing error
> path.
Won't this conflict with reserving same I/O ports? Only primary card
succeeds with reservation, so if you really want to fix this "bug", then
remove this I/O reservation from matroxfb completely, matroxfb never
touches this I/O region, it just reserves it to make sure that you won't
load vesafb and matroxfb for same head. Your fix makes use of matroxfb
with two Matrox cards in the box impossible, as on second card 0x3C0
reservation must fail as first head made reservation already.
Petr
> Signed-off-by: Darren Jenkins <darrenrjenkins@gmail.com>
>
> --- linux-2.6.16-rc4/drivers/video/matrox/matroxfb_base.c.orig 2006-02-24 23:44:10.000000000 +1100
> +++ linux-2.6.16-rc4/drivers/video/matrox/matroxfb_base.c 2006-02-27 19:26:26.000000000 +1100
> @@ -1734,7 +1734,11 @@ static int initMatrox2(WPMINFO struct bo
> #endif /* CONFIG_MTRR */
>
> if (!ACCESS_FBINFO(devflags.novga))
> - request_region(0x3C0, 32, "matrox");
> + if (!request_region(0x3C0, 32, "matrox")) {
> + printk(KERN_ERR "matroxfb: failed to request_region");
> + err = -EBUSY;
> + goto failVideoIO;
> + }
> matroxfb_g450_connect(PMINFO2);
> ACCESS_FBINFO(hw_switch->reset(PMINFO2));
>
>
>
>
[-- 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] 18+ messages in thread
* [KJ][Patch] check return value of request_irq in amiserial.c
2006-02-27 8:40 [KJ][Patch] check return value of request_region in matroxfb_base.c Darren Jenkins\
2006-03-01 7:12 ` [KJ][Patch] check return value of request region in ali5455.c Darren Jenkins\
2006-03-03 6:31 ` [KJ][Patch] check return value of request_region in Petr Vandrovec
@ 2006-03-11 13:48 ` Darren Jenkins\
2006-03-13 4:37 ` [KJ][Patch] check return value of request_irq in pcnet_cs.c Darren Jenkins\
` (13 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: Darren Jenkins\ @ 2006-03-11 13:48 UTC (permalink / raw)
To: kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 1724 bytes --]
G'day list
amiserial.c does not check the return value of request_irq @ lines 2060
and 2061 in function rs_init();
The patch below checks the return value of the two calls and cleans up
before exiting on failure.
When trying to compile this file I found that this is some built-in
amiga serial driver and isn't used on i386 (only m68k and ppc), so 1,
this isn't even compile tested and 2 the patch might not be required as
the driver init might only be used in early boot code, I don't know.
(I thought I was safe from that problem as it was in drivers/)
Signed-off-by: Darren Jenkins <darrenrjenkins@gmail.com>
--- linux-2.6.16-rc5/drivers/char/amiserial.c.orig 2006-02-28 18:55:06.000000000 +1100
+++ linux-2.6.16-rc5/drivers/char/amiserial.c 2006-03-12 00:44:55.000000000 +1100
@@ -2057,8 +2057,14 @@ static int __init rs_init(void)
local_irq_save(flags);
/* set ISRs, and then disable the rx interrupts */
- request_irq(IRQ_AMIGA_TBE, ser_tx_int, 0, "serial TX", state);
- request_irq(IRQ_AMIGA_RBF, ser_rx_int, SA_INTERRUPT, "serial RX", state);
+ if (request_irq(IRQ_AMIGA_TBE, ser_tx_int, 0, "serial TX", state) < 0)
+ {
+ goto err_irq;
+ }
+ if (request_irq(IRQ_AMIGA_RBF, ser_rx_int, SA_INTERRUPT, "serial RX", state) < 0)
+ {
+ goto err_irq_2;
+ }
/* turn off Rx and Tx interrupts */
custom.intena = IF_RBF | IF_TBE;
@@ -2078,6 +2084,14 @@ static int __init rs_init(void)
ciab.ddra &= ~(SER_DCD | SER_CTS | SER_DSR); /* inputs */
return 0;
+
+err_irq_2:
+ free_irq(IRQ_AMIGA_TBE, state);
+err_irq:
+ local_irq_restore(flags);
+ release_mem_region(CUSTOM_PHYSADDR+0x30, 4);
+ printk(KERN_WARN "rs_init could not request_irq\n");
+ return -EBUSY;
}
static __exit void rs_exit(void)
[-- 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] 18+ messages in thread
* [KJ][Patch] check return value of request_irq in pcnet_cs.c
2006-02-27 8:40 [KJ][Patch] check return value of request_region in matroxfb_base.c Darren Jenkins\
` (2 preceding siblings ...)
2006-03-11 13:48 ` [KJ][Patch] check return value of request_irq in amiserial.c Darren Jenkins\
@ 2006-03-13 4:37 ` Darren Jenkins\
2006-03-13 5:00 ` Randy.Dunlap
` (12 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: Darren Jenkins\ @ 2006-03-13 4:37 UTC (permalink / raw)
To: kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 836 bytes --]
G'day list
pcnet_cs.c called request_irq without checking its return value @ line
1036.
The patch below just checks the return value and returns -EBUSY on
failure.
This file also contains 4 space indenting so I might follow with a patch
to fix that too.
Signed-off-by: Darren Jenkins <darrenrjenkins@gmail.com>
--- linux-2.6.16-rc5/drivers/net/pcmcia/pcnet_cs.c.orig 2006-03-13 13:25:13.000000000 +1100
+++ linux-2.6.16-rc5/drivers/net/pcmcia/pcnet_cs.c 2006-03-13 13:46:07.000000000 +1100
@@ -1033,7 +1033,8 @@ static int pcnet_open(struct net_device
link->open++;
set_misc_reg(dev);
- request_irq(dev->irq, ei_irq_wrapper, SA_SHIRQ, dev_info, dev);
+ if (request_irq(dev->irq, ei_irq_wrapper, SA_SHIRQ, dev_info, dev) < 0 )
+ return -EBUSY
info->phy_id = info->eth_phy;
info->link_status = 0x00;
[-- 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] 18+ messages in thread
* Re: [KJ][Patch] check return value of request_irq in pcnet_cs.c
2006-02-27 8:40 [KJ][Patch] check return value of request_region in matroxfb_base.c Darren Jenkins\
` (3 preceding siblings ...)
2006-03-13 4:37 ` [KJ][Patch] check return value of request_irq in pcnet_cs.c Darren Jenkins\
@ 2006-03-13 5:00 ` Randy.Dunlap
2006-03-13 5:16 ` Darren Jenkins\
` (11 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: Randy.Dunlap @ 2006-03-13 5:00 UTC (permalink / raw)
To: kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 996 bytes --]
On Mon, 13 Mar 2006 15:37:33 +1100 Darren Jenkins\\ wrote:
> G'day list
>
> pcnet_cs.c called request_irq without checking its return value @ line
> 1036.
>
> The patch below just checks the return value and returns -EBUSY on
> failure.
>
> This file also contains 4 space indenting so I might follow with a patch
> to fix that too.
>
> Signed-off-by: Darren Jenkins <darrenrjenkins@gmail.com>
>
> --- linux-2.6.16-rc5/drivers/net/pcmcia/pcnet_cs.c.orig 2006-03-13 13:25:13.000000000 +1100
> +++ linux-2.6.16-rc5/drivers/net/pcmcia/pcnet_cs.c 2006-03-13 13:46:07.000000000 +1100
> @@ -1033,7 +1033,8 @@ static int pcnet_open(struct net_device
> link->open++;
>
> set_misc_reg(dev);
> - request_irq(dev->irq, ei_irq_wrapper, SA_SHIRQ, dev_info, dev);
> + if (request_irq(dev->irq, ei_irq_wrapper, SA_SHIRQ, dev_info, dev) < 0 )
> + return -EBUSY
and this built OK for you ???
---
~Randy
You can't do anything without having to do something else first.
-- Belefant's Law
[-- 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] 18+ messages in thread
* Re: [KJ][Patch] check return value of request_irq in pcnet_cs.c
2006-02-27 8:40 [KJ][Patch] check return value of request_region in matroxfb_base.c Darren Jenkins\
` (4 preceding siblings ...)
2006-03-13 5:00 ` Randy.Dunlap
@ 2006-03-13 5:16 ` Darren Jenkins\
2006-03-13 5:27 ` Randy.Dunlap
` (10 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: Darren Jenkins\ @ 2006-03-13 5:16 UTC (permalink / raw)
To: kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 809 bytes --]
On Sun, 2006-03-12 at 21:00 -0800, Randy.Dunlap wrote:
>
> and this built OK for you ???
Good point, I forgot to make it before sending, as I was still editing
the file.
Sorry about that list, here is an improved patch.
Signed-off-by: Darren Jenkins <darrenrjenkins@gmail.com>
--- linux-2.6.16-rc5/drivers/net/pcmcia/pcnet_cs.c.orig 2006-03-13 13:25:13.000000000 +1100
+++ linux-2.6.16-rc5/drivers/net/pcmcia/pcnet_cs.c.back 2006-03-13 16:10:50.000000000 +1100
@@ -1033,7 +1033,8 @@ static int pcnet_open(struct net_device
link->open++;
set_misc_reg(dev);
- request_irq(dev->irq, ei_irq_wrapper, SA_SHIRQ, dev_info, dev);
+ if (request_irq(dev->irq, ei_irq_wrapper, SA_SHIRQ, dev_info, dev) < 0 )
+ return -EBUSY;
info->phy_id = info->eth_phy;
info->link_status = 0x00;
[-- 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] 18+ messages in thread
* Re: [KJ][Patch] check return value of request_irq in pcnet_cs.c
2006-02-27 8:40 [KJ][Patch] check return value of request_region in matroxfb_base.c Darren Jenkins\
` (5 preceding siblings ...)
2006-03-13 5:16 ` Darren Jenkins\
@ 2006-03-13 5:27 ` Randy.Dunlap
2006-03-13 6:16 ` Darren Jenkins\
` (9 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: Randy.Dunlap @ 2006-03-13 5:27 UTC (permalink / raw)
To: kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 1102 bytes --]
On Mon, 13 Mar 2006 16:16:56 +1100 Darren Jenkins\\ wrote:
> On Sun, 2006-03-12 at 21:00 -0800, Randy.Dunlap wrote:
>
> >
> > and this built OK for you ???
>
> Good point, I forgot to make it before sending, as I was still editing
> the file.
> Sorry about that list, here is an improved patch.
>
> Signed-off-by: Darren Jenkins <darrenrjenkins@gmail.com>
>
> --- linux-2.6.16-rc5/drivers/net/pcmcia/pcnet_cs.c.orig 2006-03-13 13:25:13.000000000 +1100
> +++ linux-2.6.16-rc5/drivers/net/pcmcia/pcnet_cs.c.back 2006-03-13 16:10:50.000000000 +1100
> @@ -1033,7 +1033,8 @@ static int pcnet_open(struct net_device
> link->open++;
>
> set_misc_reg(dev);
> - request_irq(dev->irq, ei_irq_wrapper, SA_SHIRQ, dev_info, dev);
> + if (request_irq(dev->irq, ei_irq_wrapper, SA_SHIRQ, dev_info, dev) < 0 )
> + return -EBUSY;
>
> info->phy_id = info->eth_phy;
> info->link_status = 0x00;
Yes, that's better, but has some "improper" whitespace:
don't put a space between 0 and ')'.
---
~Randy
You can't do anything without having to do something else first.
-- Belefant's Law
[-- 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] 18+ messages in thread
* Re: [KJ][Patch] check return value of request_irq in pcnet_cs.c
2006-02-27 8:40 [KJ][Patch] check return value of request_region in matroxfb_base.c Darren Jenkins\
` (6 preceding siblings ...)
2006-03-13 5:27 ` Randy.Dunlap
@ 2006-03-13 6:16 ` Darren Jenkins\
2006-03-13 11:56 ` [KJ][Patch] check return value of request_irq in i82365.c Darren Jenkins\
` (8 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: Darren Jenkins\ @ 2006-03-13 6:16 UTC (permalink / raw)
To: kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 812 bytes --]
On Sun, 2006-03-12 at 21:27 -0800, Randy.Dunlap wrote:
>
> Yes, that's better, but has some "improper" whitespace:
> don't put a space between 0 and ')'.
>
Ok, here is an even better than the 'improved patch' patch then. :)
Signed-off-by: Darren Jenkins <darrenrjenkins@gmail.com>
--- linux-2.6.16-rc5/drivers/net/pcmcia/pcnet_cs.c.orig 2006-03-13 13:25:13.000000000 +1100
+++ linux-2.6.16-rc5/drivers/net/pcmcia/pcnet_cs.c.back 2006-03-13 17:11:54.000000000 +1100
@@ -1033,7 +1033,8 @@ static int pcnet_open(struct net_device
link->open++;
set_misc_reg(dev);
- request_irq(dev->irq, ei_irq_wrapper, SA_SHIRQ, dev_info, dev);
+ if (request_irq(dev->irq, ei_irq_wrapper, SA_SHIRQ, dev_info, dev) < 0)
+ return -EBUSY;
info->phy_id = info->eth_phy;
info->link_status = 0x00;
[-- 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] 18+ messages in thread
* [KJ][Patch] check return value of request_irq in i82365.c
2006-02-27 8:40 [KJ][Patch] check return value of request_region in matroxfb_base.c Darren Jenkins\
` (7 preceding siblings ...)
2006-03-13 6:16 ` Darren Jenkins\
@ 2006-03-13 11:56 ` Darren Jenkins\
2006-03-13 12:29 ` Tobias Klauser
` (7 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: Darren Jenkins\ @ 2006-03-13 11:56 UTC (permalink / raw)
To: kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 2276 bytes --]
G'day list
i82365.c calls request_irq without checking the return value @ line
1297.
The patch below checks the return value, and serialises the error path,
which is currently a little confusing.
This file also has 4 space indenting(what is up with pcmcia drivers?),
so I might follow with a patch fixing the coding style for it
too.(tomorrow)
This patch is compile tested.
Signed-off-by: Darren Jenkins <darrenrjenkins@gmail.com>
--- linux-2.6.16-rc5/drivers/pcmcia/i82365.c.orig 2006-03-13 17:49:26.000000000 +1100
+++ linux-2.6.16-rc5/drivers/pcmcia/i82365.c 2006-03-13 22:52:56.000000000 +1100
@@ -1264,20 +1264,17 @@ static int __init init_i82365(void)
ret = driver_register(&i82365_driver);
if (ret)
- return ret;
+ goto out;
i82365_device = platform_device_alloc("i82365", 0);
- if (i82365_device) {
- ret = platform_device_add(i82365_device);
- if (ret)
- platform_device_put(i82365_device);
- } else
- ret = -ENOMEM;
-
- if (ret) {
- driver_unregister(&i82365_driver);
- return ret;
+ if (!i82365_device) {
+ ret = -ENOMEM;
+ goto ur_out;
}
+
+ ret = platform_device_add(i82365_device);
+ if (ret)
+ goto pdp_out;
printk(KERN_INFO "Intel ISA PCIC probe: ");
sockets = 0;
@@ -1286,15 +1283,18 @@ static int __init init_i82365(void)
if (sockets == 0) {
printk("not found.\n");
- platform_device_unregister(i82365_device);
- release_region(i365_base, 2);
- driver_unregister(&i82365_driver);
- return -ENODEV;
+ ret = -ENODEV;
+ goto r_out;
}
/* Set up interrupt handler(s) */
if (grab_irq != 0)
- request_irq(cs_irq, pcic_interrupt, 0, "i82365", pcic_interrupt);
+ if (request_irq(cs_irq, pcic_interrupt, 0, "i82365", pcic_interrupt)< 0)
+ {
+ printk(KERN_ERR "init_i82365 could not request_irq");
+ ret = -EBUSY;
+ goto r_out;
+ }
/* register sockets with the pcmcia core */
for (i = 0; i < sockets; i++) {
@@ -1325,6 +1325,16 @@ static int __init init_i82365(void)
}
return 0;
+
+r_out:
+ release_region(i365_base, 2);
+ platform_device_del(i82365_device);
+pdp_out:
+ platform_device_put(i82365_device);
+ur_out:
+ driver_unregister(&i82365_driver);
+out:
+ return ret;
} /* init_i82365 */
[-- 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] 18+ messages in thread
* Re: [KJ][Patch] check return value of request_irq in i82365.c
2006-02-27 8:40 [KJ][Patch] check return value of request_region in matroxfb_base.c Darren Jenkins\
` (8 preceding siblings ...)
2006-03-13 11:56 ` [KJ][Patch] check return value of request_irq in i82365.c Darren Jenkins\
@ 2006-03-13 12:29 ` Tobias Klauser
2006-03-14 8:44 ` Darren Jenkins\
` (6 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: Tobias Klauser @ 2006-03-13 12:29 UTC (permalink / raw)
To: kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 1640 bytes --]
On 2006-03-13 at 12:56:52 +0100, Darren Jenkins" <darrenrjenkins@gmail.com> wrote:
> --- linux-2.6.16-rc5/drivers/pcmcia/i82365.c.orig 2006-03-13 17:49:26.000000000 +1100
> +++ linux-2.6.16-rc5/drivers/pcmcia/i82365.c 2006-03-13 22:52:56.000000000 +1100
> @@ -1264,20 +1264,17 @@ static int __init init_i82365(void)
>
> ret = driver_register(&i82365_driver);
> if (ret)
> - return ret;
> + goto out;
>
> i82365_device = platform_device_alloc("i82365", 0);
> - if (i82365_device) {
> - ret = platform_device_add(i82365_device);
> - if (ret)
> - platform_device_put(i82365_device);
> - } else
> - ret = -ENOMEM;
> -
> - if (ret) {
> - driver_unregister(&i82365_driver);
> - return ret;
> + if (!i82365_device) {
> + ret = -ENOMEM;
> + goto ur_out;
> }
> +
> + ret = platform_device_add(i82365_device);
> + if (ret)
> + goto pdp_out;
>
> printk(KERN_INFO "Intel ISA PCIC probe: ");
> sockets = 0;
> @@ -1286,15 +1283,18 @@ static int __init init_i82365(void)
>
> if (sockets == 0) {
> printk("not found.\n");
> - platform_device_unregister(i82365_device);
> - release_region(i365_base, 2);
> - driver_unregister(&i82365_driver);
> - return -ENODEV;
> + ret = -ENODEV;
> + goto r_out;
> }
>
> /* Set up interrupt handler(s) */
> if (grab_irq != 0)
> - request_irq(cs_irq, pcic_interrupt, 0, "i82365", pcic_interrupt);
> + if (request_irq(cs_irq, pcic_interrupt, 0, "i82365", pcic_interrupt)< 0)
> + {
^-- You might want to have this on the above line as with the if's
before. Also a space before the '<' would be good.
Cheers, Tobias
[-- 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] 18+ messages in thread
* Re: [KJ][Patch] check return value of request_irq in i82365.c
2006-02-27 8:40 [KJ][Patch] check return value of request_region in matroxfb_base.c Darren Jenkins\
` (9 preceding siblings ...)
2006-03-13 12:29 ` Tobias Klauser
@ 2006-03-14 8:44 ` Darren Jenkins\
2006-03-14 16:26 ` Randy.Dunlap
` (5 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: Darren Jenkins\ @ 2006-03-14 8:44 UTC (permalink / raw)
To: kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 3405 bytes --]
On Mon, 2006-03-13 at 13:29 +0100, Tobias Klauser wrote:
> > /* Set up interrupt handler(s) */
> > if (grab_irq != 0)
> > - request_irq(cs_irq, pcic_interrupt, 0, "i82365", pcic_interrupt);
> > + if (request_irq(cs_irq, pcic_interrupt, 0, "i82365", pcic_interrupt)< 0)
> > + {
> ^-- You might want to have this on the above line as with the if's
> before. Also a space before the '<' would be good.
>
> Cheers, Tobias
OK here is the patch with your change, doing it on two lines like this
was the best I could make it look in 80 cols.
Also I noticed another problem where isa_probe() might or might not
succeed with a request_region() meaning that we might release_region()
on something we don't own. The patch below fixes this too.
If this patch is OK I will follow with the coding style clean up.
Signed-off-by: Darren Jenkins <darrenrjenkins@gmail.com>
--- linux-2.6.16-rc5/drivers/pcmcia/i82365.c.orig 2006-03-13 17:49:26.000000000 +1100
+++ linux-2.6.16-rc5/drivers/pcmcia/i82365.c 2006-03-14 19:27:39.000000000 +1100
@@ -770,7 +770,7 @@ MODULE_DEVICE_TABLE(isapnp, id_table);
static struct pnp_dev *i82365_pnpdev;
#endif
-static void __init isa_probe(void)
+static int __init isa_probe(void)
{
int i, j, sock, k, ns, id;
kio_addr_t port;
@@ -805,7 +805,7 @@ static void __init isa_probe(void)
if (!request_region(i365_base, 2, "i82365")) {
if (sockets == 0)
printk("port conflict at %#lx\n", i365_base);
- return;
+ return -EBUSY;
}
id = identify(i365_base, 0);
@@ -844,6 +844,7 @@ static void __init isa_probe(void)
if (ns != 0) add_pcic(ns, id);
}
}
+ return 0;
}
/*====================================================================*/
@@ -1264,37 +1265,37 @@ static int __init init_i82365(void)
ret = driver_register(&i82365_driver);
if (ret)
- return ret;
+ goto out;
i82365_device = platform_device_alloc("i82365", 0);
- if (i82365_device) {
- ret = platform_device_add(i82365_device);
- if (ret)
- platform_device_put(i82365_device);
- } else
- ret = -ENOMEM;
-
- if (ret) {
- driver_unregister(&i82365_driver);
- return ret;
+ if (!i82365_device) {
+ ret = -ENOMEM;
+ goto ur_out;
}
+
+ ret = platform_device_add(i82365_device);
+ if (ret)
+ goto pdp_out;
printk(KERN_INFO "Intel ISA PCIC probe: ");
sockets = 0;
- isa_probe();
+ i = isa_probe();
if (sockets == 0) {
printk("not found.\n");
- platform_device_unregister(i82365_device);
- release_region(i365_base, 2);
- driver_unregister(&i82365_driver);
- return -ENODEV;
+ ret = -ENODEV;
+ goto r_out;
}
/* Set up interrupt handler(s) */
if (grab_irq != 0)
- request_irq(cs_irq, pcic_interrupt, 0, "i82365", pcic_interrupt);
+ if (request_irq(cs_irq, pcic_interrupt, 0, "i82365",
+ pcic_interrupt) < 0){
+ printk(KERN_ERR "init_i82365 could not request_irq");
+ ret = -EBUSY;
+ goto r_out;
+ }
/* register sockets with the pcmcia core */
for (i = 0; i < sockets; i++) {
@@ -1325,6 +1326,17 @@ static int __init init_i82365(void)
}
return 0;
+
+r_out:
+ if (i == 0)
+ release_region(i365_base, 2);
+ platform_device_del(i82365_device);
+pdp_out:
+ platform_device_put(i82365_device);
+ur_out:
+ driver_unregister(&i82365_driver);
+out:
+ return ret;
} /* init_i82365 */
[-- 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] 18+ messages in thread
* Re: [KJ][Patch] check return value of request_irq in i82365.c
2006-02-27 8:40 [KJ][Patch] check return value of request_region in matroxfb_base.c Darren Jenkins\
` (10 preceding siblings ...)
2006-03-14 8:44 ` Darren Jenkins\
@ 2006-03-14 16:26 ` Randy.Dunlap
2006-03-15 7:02 ` Darren Jenkins\
` (4 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: Randy.Dunlap @ 2006-03-14 16:26 UTC (permalink / raw)
To: kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 3517 bytes --]
On Tue, 14 Mar 2006 19:44:37 +1100 Darren Jenkins\\ wrote:
> On Mon, 2006-03-13 at 13:29 +0100, Tobias Klauser wrote:
>
> > > /* Set up interrupt handler(s) */
> > > if (grab_irq != 0)
> > > - request_irq(cs_irq, pcic_interrupt, 0, "i82365", pcic_interrupt);
> > > + if (request_irq(cs_irq, pcic_interrupt, 0, "i82365", pcic_interrupt)< 0)
> > > + {
> > ^-- You might want to have this on the above line as with the if's
> > before. Also a space before the '<' would be good.
> >
> > Cheers, Tobias
>
> OK here is the patch with your change, doing it on two lines like this
> was the best I could make it look in 80 cols.
> Also I noticed another problem where isa_probe() might or might not
> succeed with a request_region() meaning that we might release_region()
> on something we don't own. The patch below fixes this too.
>
> If this patch is OK I will follow with the coding style clean up.
>
> Signed-off-by: Darren Jenkins <darrenrjenkins@gmail.com>
>
> --- linux-2.6.16-rc5/drivers/pcmcia/i82365.c.orig 2006-03-13 17:49:26.000000000 +1100
> +++ linux-2.6.16-rc5/drivers/pcmcia/i82365.c 2006-03-14 19:27:39.000000000 +1100
> @@ -770,7 +770,7 @@ MODULE_DEVICE_TABLE(isapnp, id_table);
> static struct pnp_dev *i82365_pnpdev;
> #endif
>
> -static void __init isa_probe(void)
> +static int __init isa_probe(void)
> {
> int i, j, sock, k, ns, id;
> kio_addr_t port;
> @@ -805,7 +805,7 @@ static void __init isa_probe(void)
> if (!request_region(i365_base, 2, "i82365")) {
> if (sockets == 0)
> printk("port conflict at %#lx\n", i365_base);
> - return;
> + return -EBUSY;
> }
>
> id = identify(i365_base, 0);
> @@ -844,6 +844,7 @@ static void __init isa_probe(void)
> if (ns != 0) add_pcic(ns, id);
> }
> }
> + return 0;
> }
>
> /*====================================================================*/
> @@ -1264,37 +1265,37 @@ static int __init init_i82365(void)
>
> ret = driver_register(&i82365_driver);
> if (ret)
> - return ret;
> + goto out;
>
> i82365_device = platform_device_alloc("i82365", 0);
> - if (i82365_device) {
> - ret = platform_device_add(i82365_device);
> - if (ret)
> - platform_device_put(i82365_device);
> - } else
> - ret = -ENOMEM;
> -
> - if (ret) {
> - driver_unregister(&i82365_driver);
> - return ret;
> + if (!i82365_device) {
> + ret = -ENOMEM;
> + goto ur_out;
> }
> +
> + ret = platform_device_add(i82365_device);
> + if (ret)
> + goto pdp_out;
>
> printk(KERN_INFO "Intel ISA PCIC probe: ");
> sockets = 0;
>
> - isa_probe();
> + i = isa_probe();
>
> if (sockets == 0) {
> printk("not found.\n");
> - platform_device_unregister(i82365_device);
> - release_region(i365_base, 2);
> - driver_unregister(&i82365_driver);
> - return -ENODEV;
> + ret = -ENODEV;
> + goto r_out;
> }
>
> /* Set up interrupt handler(s) */
> if (grab_irq != 0)
> - request_irq(cs_irq, pcic_interrupt, 0, "i82365", pcic_interrupt);
> + if (request_irq(cs_irq, pcic_interrupt, 0, "i82365",
> + pcic_interrupt) < 0){
> + printk(KERN_ERR "init_i82365 could not request_irq");
> + ret = -EBUSY;
> + goto r_out;
> + }
well.... if you have to make any more changes, add a space after
the "< 0)" and before the opening '{'.
The "grammar" isn't tough IMO. Maybe you are just used to not
using whitespace?
---
~Randy
You can't do anything without having to do something else first.
-- Belefant's Law
[-- 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] 18+ messages in thread
* Re: [KJ][Patch] check return value of request_irq in i82365.c
2006-02-27 8:40 [KJ][Patch] check return value of request_region in matroxfb_base.c Darren Jenkins\
` (11 preceding siblings ...)
2006-03-14 16:26 ` Randy.Dunlap
@ 2006-03-15 7:02 ` Darren Jenkins\
2006-03-15 7:32 ` Nishanth Aravamudan
` (3 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: Darren Jenkins\ @ 2006-03-15 7:02 UTC (permalink / raw)
To: kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 3126 bytes --]
On Tue, 2006-03-14 at 08:26 -0800, Randy.Dunlap wrote:
> well.... if you have to make any more changes, add a space after
> the "< 0)" and before the opening '{'.
> The "grammar" isn't tough IMO. Maybe you are just used to not
> using whitespace?
White-space lol, I usually use a lot more, but I have never been
restricted to 80 columns before and I'm not used to using the K&R coding
style that you old folks all seem to use. :P
Here is an improved patch. I guess I will start with the style cleanups,
unless there are anymore problems.
Signed-off-by: Darren Jenkins <darrenrjenkins@gmail.com>
--- linux-2.6.16-rc6/drivers/pcmcia/i82365.c.orig 2006-03-15 17:34:45.000000000 +1100
+++ linux-2.6.16-rc6/drivers/pcmcia/i82365.c 2006-03-15 17:37:59.000000000 +1100
@@ -770,7 +770,7 @@ MODULE_DEVICE_TABLE(isapnp, id_table);
static struct pnp_dev *i82365_pnpdev;
#endif
-static void __init isa_probe(void)
+static int __init isa_probe(void)
{
int i, j, sock, k, ns, id;
kio_addr_t port;
@@ -805,7 +805,7 @@ static void __init isa_probe(void)
if (!request_region(i365_base, 2, "i82365")) {
if (sockets == 0)
printk("port conflict at %#lx\n", i365_base);
- return;
+ return -EBUSY;
}
id = identify(i365_base, 0);
@@ -844,6 +844,7 @@ static void __init isa_probe(void)
if (ns != 0) add_pcic(ns, id);
}
}
+ return 0;
}
/*====================================================================*/
@@ -1264,37 +1265,37 @@ static int __init init_i82365(void)
ret = driver_register(&i82365_driver);
if (ret)
- return ret;
+ goto out;
i82365_device = platform_device_alloc("i82365", 0);
- if (i82365_device) {
- ret = platform_device_add(i82365_device);
- if (ret)
- platform_device_put(i82365_device);
- } else
- ret = -ENOMEM;
-
- if (ret) {
- driver_unregister(&i82365_driver);
- return ret;
+ if (!i82365_device) {
+ ret = -ENOMEM;
+ goto ur_out;
}
+
+ ret = platform_device_add(i82365_device);
+ if (ret)
+ goto pdp_out;
printk(KERN_INFO "Intel ISA PCIC probe: ");
sockets = 0;
- isa_probe();
+ i = isa_probe();
if (sockets == 0) {
printk("not found.\n");
- platform_device_unregister(i82365_device);
- release_region(i365_base, 2);
- driver_unregister(&i82365_driver);
- return -ENODEV;
+ ret = -ENODEV;
+ goto r_out;
}
/* Set up interrupt handler(s) */
if (grab_irq != 0)
- request_irq(cs_irq, pcic_interrupt, 0, "i82365", pcic_interrupt);
+ if (request_irq(cs_irq, pcic_interrupt, 0, "i82365",
+ pcic_interrupt) < 0) {
+ printk(KERN_ERR "init_i82365 could not request_irq");
+ ret = -EBUSY;
+ goto r_out;
+ }
/* register sockets with the pcmcia core */
for (i = 0; i < sockets; i++) {
@@ -1325,6 +1326,17 @@ static int __init init_i82365(void)
}
return 0;
+
+r_out:
+ if (i == 0)
+ release_region(i365_base, 2);
+ platform_device_del(i82365_device);
+pdp_out:
+ platform_device_put(i82365_device);
+ur_out:
+ driver_unregister(&i82365_driver);
+out:
+ return ret;
} /* init_i82365 */
[-- 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] 18+ messages in thread
* Re: [KJ][Patch] check return value of request_irq in i82365.c
2006-02-27 8:40 [KJ][Patch] check return value of request_region in matroxfb_base.c Darren Jenkins\
` (12 preceding siblings ...)
2006-03-15 7:02 ` Darren Jenkins\
@ 2006-03-15 7:32 ` Nishanth Aravamudan
2006-03-15 12:46 ` [KJ][Patch] check return value of request_region and request_irq Darren Jenkins\
` (2 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: Nishanth Aravamudan @ 2006-03-15 7:32 UTC (permalink / raw)
To: kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 776 bytes --]
On 15.03.2006 [18:02:59 +1100], Darren Jenkins" wrote:
> On Tue, 2006-03-14 at 08:26 -0800, Randy.Dunlap wrote:
>
> > well.... if you have to make any more changes, add a space after
> > the "< 0)" and before the opening '{'.
> > The "grammar" isn't tough IMO. Maybe you are just used to not
> > using whitespace?
>
> White-space lol, I usually use a lot more, but I have never been
> restricted to 80 columns before and I'm not used to using the K&R coding
> style that you old folks all seem to use. :P
>
> Here is an improved patch. I guess I will start with the style cleanups,
> unless there are anymore problems.
Without looking at the thread, it's now impossible to know what the does
:) Always keep the description tag with each patch you send out.
Thanks,
Nish
[-- 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] 18+ messages in thread
* [KJ][Patch] check return value of request_region and request_irq
2006-02-27 8:40 [KJ][Patch] check return value of request_region in matroxfb_base.c Darren Jenkins\
` (13 preceding siblings ...)
2006-03-15 7:32 ` Nishanth Aravamudan
@ 2006-03-15 12:46 ` Darren Jenkins\
2006-03-15 12:57 ` Tobias Klauser
2006-03-16 10:45 ` [KJ][Patch] check return value of request_region and Darren Jenkins\
16 siblings, 0 replies; 18+ messages in thread
From: Darren Jenkins\ @ 2006-03-15 12:46 UTC (permalink / raw)
To: kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 3347 bytes --]
G'day list,
m32r_pcc.c calls request_irq without checking its return value @ line
346, and request_region without checking its return value @ line 333.
The patch below checks the two return values, and serialises the error
path of init_m32r_pcc().
Note this driver is for the m32r arch so this patch is not even compile
tested, it might not even come close to compiling(I would appreciate a
report either way). Also this may be a low level driver that will never
fail a request_region or request_irq, I don't know.
(I figured why the hell not send a patch?)
Signed-off-by: Darren Jenkins <darrenrjenkins@gmail.com>
--- linux-2.6.16-rc6/drivers/pcmcia/m32r_pcc.c.orig 2006-03-14 20:55:44.000000000 +1100
+++ linux-2.6.16-rc6/drivers/pcmcia/m32r_pcc.c 2006-03-15 23:32:00.000000000 +1100
@@ -312,7 +312,7 @@ static int __init is_alive(u_short sock)
return 0;
}
-static void add_pcc_socket(ulong base, int irq, ulong mapaddr, kio_addr_t ioaddr)
+static int add_pcc_socket(ulong base, int irq, ulong mapaddr, kio_addr_t ioaddr)
{
pcc_socket_t *t = &socket[pcc_sockets];
@@ -330,7 +330,8 @@ static void add_pcc_socket(ulong base, i
/* add pcc */
if (t->base > 0) {
- request_region(t->base, 0x20, "m32r-pcc");
+ if ( request_region(t->base, 0x20, "m32r-pcc")== 0)
+ return -EBUSY;
}
printk(KERN_INFO " %s ", pcc[pcc_sockets].name);
@@ -343,11 +344,13 @@ static void add_pcc_socket(ulong base, i
t->socket.irq_mask = 0;
t->socket.pci_irq = 2 + pcc_sockets; /* XXX */
- request_irq(irq, pcc_interrupt, 0, "m32r-pcc", pcc_interrupt);
+ if (request_irq(irq, pcc_interrupt, 0, "m32r-pcc", pcc_interrupt) < 0) {
+ release_region(t->base, 0x20);
+ return -EBUSY;
pcc_sockets++;
- return;
+ return 0;
}
@@ -692,32 +695,38 @@ static struct platform_device pcc_device
static int __init init_m32r_pcc(void)
{
int i, ret;
+ pcc_socket_t *t;
ret = driver_register(&pcc_driver);
if (ret)
- return ret;
+ goto out;
ret = platform_device_register(&pcc_device);
- if (ret){
- driver_unregister(&pcc_driver);
- return ret;
- }
+ if (ret)
+ goto pdr_out;
+
printk(KERN_INFO "m32r PCC probe:\n");
pcc_sockets = 0;
- add_pcc_socket(M32R_PCC0_BASE, PCC0_IRQ, M32R_PCC0_MAPBASE, 0x1000);
+ ret = add_pcc_socket(M32R_PCC0_BASE, PCC0_IRQ,
+ M32R_PCC0_MAPBASE, 0x1000);
+ if (ret)
+ goto ap_out;
#ifdef CONFIG_M32RPCC_SLOT2
- add_pcc_socket(M32R_PCC1_BASE, PCC1_IRQ, M32R_PCC1_MAPBASE, 0x2000);
+ ret = add_pcc_socket(M32R_PCC1_BASE, PCC1_IRQ,
+ M32R_PCC1_MAPBASE, 0x2000);
+ if (ret)
+ goto aps_out;
+
#endif
if (pcc_sockets == 0) {
printk("socket is not found.\n");
- platform_device_unregister(&pcc_device);
- driver_unregister(&pcc_driver);
- return -ENODEV;
+ ret = -ENODEV;
+ goto ns_out;
}
/* Set up interrupt handler(s) */
@@ -750,6 +759,25 @@ static int __init init_m32r_pcc(void)
}
return 0;
+
+
+ns_out:
+#ifdef CONFIG_M32RPCC_SLOT2
+ free_irq(PCC1_IRQ, pcc_interrupt);
+ t = &socket[--pcc_sockets];
+ release_region(t->base, 0x20);
+#endif
+aps_out:
+ free_irq(PCC0_IRQ, pcc_interrupt);
+ t = &socket[--pcc_sockets];
+ release_region(t->base, 0x20);
+ap_out:
+ platform_device_unregister(&pcc_device);
+pdr_out:
+ driver_unregister(&pcc_driver);
+out:
+ return ret;
+
} /* init_m32r_pcc */
static void __exit exit_m32r_pcc(void)
[-- 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] 18+ messages in thread
* Re: [KJ][Patch] check return value of request_region and request_irq
2006-02-27 8:40 [KJ][Patch] check return value of request_region in matroxfb_base.c Darren Jenkins\
` (14 preceding siblings ...)
2006-03-15 12:46 ` [KJ][Patch] check return value of request_region and request_irq Darren Jenkins\
@ 2006-03-15 12:57 ` Tobias Klauser
2006-03-16 10:45 ` [KJ][Patch] check return value of request_region and Darren Jenkins\
16 siblings, 0 replies; 18+ messages in thread
From: Tobias Klauser @ 2006-03-15 12:57 UTC (permalink / raw)
To: kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 816 bytes --]
On 2006-03-15 at 13:46:54 +0100, Darren Jenkins" <darrenrjenkins@gmail.com> wrote:
> --- linux-2.6.16-rc6/drivers/pcmcia/m32r_pcc.c.orig 2006-03-14 20:55:44.000000000 +1100
> +++ linux-2.6.16-rc6/drivers/pcmcia/m32r_pcc.c 2006-03-15 23:32:00.000000000 +1100
> @@ -312,7 +312,7 @@ static int __init is_alive(u_short sock)
> return 0;
> }
>
> -static void add_pcc_socket(ulong base, int irq, ulong mapaddr, kio_addr_t ioaddr)
> +static int add_pcc_socket(ulong base, int irq, ulong mapaddr, kio_addr_t ioaddr)
> {
> pcc_socket_t *t = &socket[pcc_sockets];
>
> @@ -330,7 +330,8 @@ static void add_pcc_socket(ulong base, i
>
> /* add pcc */
> if (t->base > 0) {
> - request_region(t->base, 0x20, "m32r-pcc");
> + if ( request_region(t->base, 0x20, "m32r-pcc")== 0)
Whitespaces again
Cheers, Tobias
[-- 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] 18+ messages in thread
* Re: [KJ][Patch] check return value of request_region and
2006-02-27 8:40 [KJ][Patch] check return value of request_region in matroxfb_base.c Darren Jenkins\
` (15 preceding siblings ...)
2006-03-15 12:57 ` Tobias Klauser
@ 2006-03-16 10:45 ` Darren Jenkins\
16 siblings, 0 replies; 18+ messages in thread
From: Darren Jenkins\ @ 2006-03-16 10:45 UTC (permalink / raw)
To: kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 3709 bytes --]
On Wed, 2006-03-15 at 13:57 +0100, Tobias Klauser wrote:
>
> > @@ -330,7 +330,8 @@ static void add_pcc_socket(ulong base, i
> >
> > /* add pcc */
> > if (t->base > 0) {
> > - request_region(t->base, 0x20, "m32r-pcc");
> > + if ( request_region(t->base, 0x20, "m32r-pcc")== 0)
>
> Whitespaces again
>
> Cheers, Tobias
Thanks Tobias.
Here is an updated patch.
m32r_pcc.c calls request_irq without checking its return value @ line
346, and request_region without checking its return value @ line 333.
The patch below checks the two return values, and serialises the error
path of init_m32r_pcc().
Note this driver is for the m32r arch so this patch is not even compile
tested, it might not even come close to compiling(I would appreciate a
report either way). Also this may be a low level driver that will never
fail a request_region or request_irq, I don't know.
(I figured why the hell not send a patch?)
Signed-off-by: Darren Jenkins <darrenrjenkins@gmail.com>
--- linux-2.6.16-rc6/drivers/pcmcia/m32r_pcc.c.orig 2006-03-14 20:55:44.000000000 +1100
+++ linux-2.6.16-rc6/drivers/pcmcia/m32r_pcc.c 2006-03-16 21:35:01.000000000 +1100
@@ -312,7 +312,7 @@ static int __init is_alive(u_short sock)
return 0;
}
-static void add_pcc_socket(ulong base, int irq, ulong mapaddr, kio_addr_t ioaddr)
+static int add_pcc_socket(ulong base, int irq, ulong mapaddr, kio_addr_t ioaddr)
{
pcc_socket_t *t = &socket[pcc_sockets];
@@ -330,7 +330,8 @@ static void add_pcc_socket(ulong base, i
/* add pcc */
if (t->base > 0) {
- request_region(t->base, 0x20, "m32r-pcc");
+ if (request_region(t->base, 0x20, "m32r-pcc") == 0)
+ return -EBUSY;
}
printk(KERN_INFO " %s ", pcc[pcc_sockets].name);
@@ -343,11 +344,13 @@ static void add_pcc_socket(ulong base, i
t->socket.irq_mask = 0;
t->socket.pci_irq = 2 + pcc_sockets; /* XXX */
- request_irq(irq, pcc_interrupt, 0, "m32r-pcc", pcc_interrupt);
+ if (request_irq(irq, pcc_interrupt, 0, "m32r-pcc", pcc_interrupt) < 0) {
+ release_region(t->base, 0x20);
+ return -EBUSY;
pcc_sockets++;
- return;
+ return 0;
}
@@ -692,32 +695,38 @@ static struct platform_device pcc_device
static int __init init_m32r_pcc(void)
{
int i, ret;
+ pcc_socket_t *t;
ret = driver_register(&pcc_driver);
if (ret)
- return ret;
+ goto out;
ret = platform_device_register(&pcc_device);
- if (ret){
- driver_unregister(&pcc_driver);
- return ret;
- }
+ if (ret)
+ goto pdr_out;
+
printk(KERN_INFO "m32r PCC probe:\n");
pcc_sockets = 0;
- add_pcc_socket(M32R_PCC0_BASE, PCC0_IRQ, M32R_PCC0_MAPBASE, 0x1000);
+ ret = add_pcc_socket(M32R_PCC0_BASE, PCC0_IRQ,
+ M32R_PCC0_MAPBASE, 0x1000);
+ if (ret)
+ goto ap_out;
#ifdef CONFIG_M32RPCC_SLOT2
- add_pcc_socket(M32R_PCC1_BASE, PCC1_IRQ, M32R_PCC1_MAPBASE, 0x2000);
+ ret = add_pcc_socket(M32R_PCC1_BASE, PCC1_IRQ,
+ M32R_PCC1_MAPBASE, 0x2000);
+ if (ret)
+ goto aps_out;
+
#endif
if (pcc_sockets == 0) {
printk("socket is not found.\n");
- platform_device_unregister(&pcc_device);
- driver_unregister(&pcc_driver);
- return -ENODEV;
+ ret = -ENODEV;
+ goto ns_out;
}
/* Set up interrupt handler(s) */
@@ -750,6 +759,25 @@ static int __init init_m32r_pcc(void)
}
return 0;
+
+
+ns_out:
+#ifdef CONFIG_M32RPCC_SLOT2
+ free_irq(PCC1_IRQ, pcc_interrupt);
+ t = &socket[--pcc_sockets];
+ release_region(t->base, 0x20);
+#endif
+aps_out:
+ free_irq(PCC0_IRQ, pcc_interrupt);
+ t = &socket[--pcc_sockets];
+ release_region(t->base, 0x20);
+ap_out:
+ platform_device_unregister(&pcc_device);
+pdr_out:
+ driver_unregister(&pcc_driver);
+out:
+ return ret;
+
} /* init_m32r_pcc */
static void __exit exit_m32r_pcc(void)
[-- 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] 18+ messages in thread
end of thread, other threads:[~2006-03-16 10:45 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-02-27 8:40 [KJ][Patch] check return value of request_region in matroxfb_base.c Darren Jenkins\
2006-03-01 7:12 ` [KJ][Patch] check return value of request region in ali5455.c Darren Jenkins\
2006-03-03 6:31 ` [KJ][Patch] check return value of request_region in Petr Vandrovec
2006-03-11 13:48 ` [KJ][Patch] check return value of request_irq in amiserial.c Darren Jenkins\
2006-03-13 4:37 ` [KJ][Patch] check return value of request_irq in pcnet_cs.c Darren Jenkins\
2006-03-13 5:00 ` Randy.Dunlap
2006-03-13 5:16 ` Darren Jenkins\
2006-03-13 5:27 ` Randy.Dunlap
2006-03-13 6:16 ` Darren Jenkins\
2006-03-13 11:56 ` [KJ][Patch] check return value of request_irq in i82365.c Darren Jenkins\
2006-03-13 12:29 ` Tobias Klauser
2006-03-14 8:44 ` Darren Jenkins\
2006-03-14 16:26 ` Randy.Dunlap
2006-03-15 7:02 ` Darren Jenkins\
2006-03-15 7:32 ` Nishanth Aravamudan
2006-03-15 12:46 ` [KJ][Patch] check return value of request_region and request_irq Darren Jenkins\
2006-03-15 12:57 ` Tobias Klauser
2006-03-16 10:45 ` [KJ][Patch] check return value of request_region and Darren Jenkins\
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.