Skip to contentSkip to conversation

drivers/parisc/superio.c: check return values of

PATCH In review Richard 3 participants 4 messages Jul 10, 2006 – Jul 11, 2006

Subsystems (2)
Kernel janitors PA-RISC architecture
lore
Share & tools
lore

Richard

author
#0 lore raw

PATCH 1/1 drivers/parisc/superio.c: check return values of

Check return values of request region and act accordingly

Signed-off-by Richard van Berkum <h.vanberkum at chello.nl>

---

My second attempt to correct some request_region code. Any input is welcome.

 drivers/parisc/superio.c |   31 +++++++++++++++++++++++++++----
 1 file changed, 27 insertions(+), 4 deletions(-)
drivers/parisc/superio.c +27 -4
@@ -189,9 +189,20 @@ superio_init(struct pci_dev *pcidev)
 	sio->acpi_base &= ~1;
 	printk(KERN_INFO PFX "ACPI at 0x%x\n", sio->acpi_base);
 
-	request_region (IC_PIC1, 0x1f, "pic1");
-	request_region (IC_PIC2, 0x1f, "pic2");
-	request_region (sio->acpi_base, 0x1f, "acpi");
+	if (!(request_region (IC_PIC1, 0x1f, "pic1"))) {
+		printk(KERN_ERR "can't get I/O %x\n", IC_PIC1);
+		goto err1;
+	}
+	
+	if(!(request_region (IC_PIC2, 0x1f, "pic2"))) {
+		printk(KERN_ERR "can't get I/O %x\n", IC_PIC2);
+		goto err2;
+	}
+	
+	if(!(request_region (sio->acpi_base, 0x1f, "acpi"))) {
+		printk(KERN_ERR "can't get I/O %x\n", sio->acpi_base);
+		goto err3;
+	}
 
 	/* Enable the legacy I/O function */
 	pci_read_config_word (pdev, PCI_COMMAND, &word);

@@ -276,10 +287,22 @@ superio_init(struct pci_dev *pcidev)
 
 		printk(KERN_ERR PFX "could not get irq\n");
 		BUG();
-		return;
+		
+		/*Free resources previously requested*/
+		goto err3;
 	}
 
 	sio->suckyio_irq_enabled = 1;
+
+	return;
+
+
+err3:	release_region(IC_PIC2, 0x1f);
+
+err2:	release_region(IC_PIC1, 0x1f);
+
+err1:	return; 
+
 }
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_NS, PCI_DEVICE_ID_NS_87560_LIO, superio_init);

_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors

#1 lore raw

drivers/parisc/superio.c superio_init(struct pci_dev *pcidev) · line 194

Quoted 5 lines · Richard

Check return values of request region and act accordingly

drivers/parisc/superio.c +0 -3
-	request_region (IC_PIC1, 0x1f, "pic1");
-	request_region (IC_PIC2, 0x1f, "pic2");
-	request_region (sio->acpi_base, 0x1f, "acpi");

This function only runs if it's in a parisc machine with a superio device. It's virtually impossible for this to fail. I've made it just BUG_ON(!request_region(...)); instead of bothering with any kind of error handling.
_______________________________________________
Kernel-janitors mailing list [email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors

#2 lore raw
In reply to Richard
Quoted 4 lines · On Tue, 11 Jul 2006 04:04:42 +0200 Richard wrote:

Check return values of request region and act accordingly

My second attempt to correct some request_region code. Any input is welcome.

OK...

Quoted 7 lines
 drivers/parisc/superio.c |   31 +++++++++++++++++++++++++++----
 1 file changed, 27 insertions(+), 4 deletions(-)

drivers/parisc/superio.c superio_init(struct pci_dev *pcidev) · line 192

Quoted 7 lines
drivers/parisc/superio.c +1 -3
@@ -189,9 +189,20 @@ superio_init(struct pci_dev *pcidev)
 	sio->acpi_base &= ~1;
 	printk(KERN_INFO PFX "ACPI at 0x%x\n", sio->acpi_base);
 
-	request_region (IC_PIC1, 0x1f, "pic1");
-	request_region (IC_PIC2, 0x1f, "pic2");
-	request_region (sio->acpi_base, 0x1f, "acpi");
+	if (!(request_region (IC_PIC1, 0x1f, "pic1"))) {

Coding style: no extra parens (like after !).
no space after function name.

drivers/parisc/superio.c superio_init(struct pci_dev *pcidev) · line 197

Quoted 5 lines
drivers/parisc/superio.c +5 -0
+		printk(KERN_ERR "can't get I/O %x\n", IC_PIC1);
+		goto err1;
+	}
+	
+	if(!(request_region (IC_PIC2, 0x1f, "pic2"))) {

space after "if". drop "!()" parens.
no space after function name.

drivers/parisc/superio.c superio_init(struct pci_dev *pcidev) · line 202

Quoted 5 lines
drivers/parisc/superio.c +5 -0
+		printk(KERN_ERR "can't get I/O %x\n", IC_PIC2);
+		goto err2;
+	}
+	
+	if(!(request_region (sio->acpi_base, 0x1f, "acpi"))) {

ditto

drivers/parisc/superio.c superio_init(struct pci_dev *pcidev) · line 291

Quoted 13 lines
drivers/parisc/superio.c +5 -1
+		printk(KERN_ERR "can't get I/O %x\n", sio->acpi_base);
+		goto err3;
+	}
 
 	/* Enable the legacy I/O function */
 	pci_read_config_word (pdev, PCI_COMMAND, &word);

@@ -276,10 +287,22 @@ superio_init(struct pci_dev *pcidev)
 
 		printk(KERN_ERR PFX "could not get irq\n");
 		BUG();
-		return;
+		
+		/*Free resources previously requested*/

See above /* comment for spacing.

Quoted 5 lines
drivers/parisc/superio.c +2 -0
+		goto err3;
 	}
 
 	sio->suckyio_irq_enabled = 1;
+

drivers/parisc/superio.c superio_init(struct pci_dev *pcidev) · line 306

Quoted 10 lines
drivers/parisc/superio.c +9 -0
+	return;
+
+
+err3:	release_region(IC_PIC2, 0x1f);
+
+err2:	release_region(IC_PIC1, 0x1f);
+
+err1:	return; 
+
 }

Put labels and statements on separate lines.

---
~Randy
_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors

author
#3 lore raw
In reply to Richard

Randy.Dunlap wrote:

Hello Randy,

Quoted 4 lines

drivers/parisc/superio.c superio_init(struct pci_dev *pcidev) · line 192

Quoted 1 line
drivers/parisc/superio.c +1 -0
+	if (!(request_region (IC_PIC1, 0x1f, "pic1"))) {

Coding style: no extra parens (like after !).
no space after function name.

The space must be a mistake. About the parens I didn't know. Thanks

Quoted 9 lines

drivers/parisc/superio.c superio_init(struct pci_dev *pcidev) · line 197

Quoted 6 lines
drivers/parisc/superio.c +5 -0
+		printk(KERN_ERR "can't get I/O %x\n", IC_PIC1);
+		goto err1;
+	}
+	
+	if(!(request_region (IC_PIC2, 0x1f, "pic2"))) {

space after "if". drop "!()" parens.
no space after function name.

Clear :)

Quoted 11 lines

drivers/parisc/superio.c superio_init(struct pci_dev *pcidev) · line 291

Quoted 9 lines
drivers/parisc/superio.c +2 -1
 	/* Enable the legacy I/O function */
 	pci_read_config_word (pdev, PCI_COMMAND, &word);

@@ -276,10 +287,22 @@ superio_init(struct pci_dev *pcidev)
 
 		printk(KERN_ERR PFX "could not get irq\n");
 		BUG();
-		return;
+		
+		/*Free resources previously requested*/

See above /* comment for spacing.

I see for clarity I gues. I'll try to remember.

Quoted 5 lines

drivers/parisc/superio.c superio_init(struct pci_dev *pcidev) · line 306

Quoted 3 lines
drivers/parisc/superio.c +2 -0
+err1:	return; 
+
 }

Put labels and statements on separate lines.

Ok. That was some good input on coding style.Thanks, but it's no use to continue on this with regard to the previous post

Quoted 4 lines

---
~Randy

Richard

_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors