Skip to contentSkip to conversation

HiSax: fix error checking for hisax_register()]

PATCH In review Martin Bachem 2 participants 2 messages May 21, 2007

lore
Share & tools
lore

Martin Bachem

author
#0 lore raw

PATCH 1/1 HiSax: fix error checking for hisax_register()]

Signed-off-by Alan Stern <[email protected]>
Signed-off-by Martin Bachem <[email protected]>
Acked-by Karsten Keil <[email protected]>

This patch (as875) adds error-checking to the callers of hisax_register(). It also changes an error pathway in that routine, making it return an error code rather than 0.

This fixes Bugzilla #7960.

Signed-off-by Alan Stern <[email protected]>

---

drivers/isdn/hisax/config.c +1 -1
Index: usb-2.6/drivers/isdn/hisax/config.c
===================================================================
@@ -1551,7 +1551,7 @@ int hisax_register(struct hisax_d_if *hi
 	if (retval == 0) { // yuck
 		cards[i].typ = 0;
 		nrcards--;
-		return retval;
+		return -EINVAL;
 	}
 	cs = cards[i].cs;
 	hisax_d_if->cs = cs;
drivers/isdn/hisax/hfc_usb.c +5 -1
Index: usb-2.6/drivers/isdn/hisax/hfc_usb.c
===================================================================
@@ -1306,7 +1306,11 @@ usb_init(hfcusb_data * hfc)
 	}
 	/* default Prot: EURO ISDN, should be a module_param */
 	hfc->protocol = 2;
-	hisax_register(&hfc->d_if, p_b_if, "hfc_usb", hfc->protocol);
+	i = hisax_register(&hfc->d_if, p_b_if, "hfc_usb", hfc->protocol);
+	if (i) {
+		printk(KERN_INFO "HFC-S USB: hisax_register -> %d\n", i);
+		return i;
+	}
 
 #ifdef CONFIG_HISAX_DEBUG
 	hfc_debug = debug;
drivers/isdn/hisax/hisax_fcpcipnp.c +5 -1
Index: usb-2.6/drivers/isdn/hisax/hisax_fcpcipnp.c
===================================================================
@@ -859,7 +859,11 @@ new_adapter(void)
 	for (i = 0; i < 2; i++)
 		b_if[i] = &adapter->bcs[i].b_if;
 
-	hisax_register(&adapter->isac.hisax_d_if, b_if, "fcpcipnp", protocol);
+	if (hisax_register(&adapter->isac.hisax_d_if, b_if, "fcpcipnp",
+			protocol) != 0) {
+		kfree(adapter);
+		adapter = NULL;
+	}
 
 	return adapter;
 }
drivers/isdn/hisax/st5481_init.c +6 -1
Index: usb-2.6/drivers/isdn/hisax/st5481_init.c
===================================================================
@@ -107,12 +107,17 @@ static int probe_st5481(struct usb_inter
 	for (i = 0; i < 2; i++)
 		b_if[i] = &adapter->bcs[i].b_if;
 
-	hisax_register(&adapter->hisax_d_if, b_if, "st5481_usb", protocol);
+	if (hisax_register(&adapter->hisax_d_if, b_if, "st5481_usb",
+			protocol) != 0)
+		goto err_b1;
+
 	st5481_start(adapter);
 
 	usb_set_intfdata(intf, adapter);
 	return 0;
 
+ err_b1:
+	st5481_release_b(&adapter->bcs[1]);
  err_b:
 	st5481_release_b(&adapter->bcs[0]);
  err_d:

This patch (as876) improves the USB API usage in the HiSax drivers:

Use usb_kill_urb() rather than usb_unlink_urb() when shutting down.

Remove unnecessary checks for NULL pointers.

Don't initialize urb->lock, because it is private to usbcore.

Signed-off-by Alan Stern <[email protected]>

---

drivers/isdn/hisax/hfc_usb.c +13 -24
Index: usb-2.6/drivers/isdn/hisax/hfc_usb.c
===================================================================
@@ -486,7 +486,6 @@ fill_isoc_urb(struct urb *urb, struct us
 {
 	int k;
 
-	spin_lock_init(&urb->lock);
 	urb->dev = dev;
 	urb->pipe = pipe;
 	urb->complete = complete;

@@ -579,16 +578,14 @@ stop_isoc_chain(usb_fifo * fifo)
 			    "HFC-S USB: Stopping iso chain for fifo %i.%i",
 			    fifo->fifonum, i);
 #endif
-			usb_unlink_urb(fifo->iso[i].purb);
+			usb_kill_urb(fifo->iso[i].purb);
 			usb_free_urb(fifo->iso[i].purb);
 			fifo->iso[i].purb = NULL;
 		}
 	}
-	if (fifo->urb) {
-		usb_unlink_urb(fifo->urb);
-		usb_free_urb(fifo->urb);
-		fifo->urb = NULL;
-	}
+	usb_kill_urb(fifo->urb);
+	usb_free_urb(fifo->urb);
+	fifo->urb = NULL;
 	fifo->active = 0;
 }
 

@@ -1631,11 +1628,9 @@ hfc_usb_probe(struct usb_interface *intf
 #endif
 			/* init the chip and register the driver */
 			if (usb_init(context)) {
-				if (context->ctrl_urb) {
-					usb_unlink_urb(context->ctrl_urb);
-					usb_free_urb(context->ctrl_urb);
-					context->ctrl_urb = NULL;
-				}
+				usb_kill_urb(context->ctrl_urb);
+				usb_free_urb(context->ctrl_urb);
+				context->ctrl_urb = NULL;
 				kfree(context);
 				return (-EIO);
 			}

@@ -1687,21 +1682,15 @@ hfc_usb_disconnect(struct usb_interface
 				    i);
 #endif
 			}
-			if (context->fifos[i].urb) {
-				usb_unlink_urb(context->fifos[i].urb);
-				usb_free_urb(context->fifos[i].urb);
-				context->fifos[i].urb = NULL;
-			}
+			usb_kill_urb(context->fifos[i].urb);
+			usb_free_urb(context->fifos[i].urb);
+			context->fifos[i].urb = NULL;
 		}
 		context->fifos[i].active = 0;
 	}
-	/* wait for all URBS to terminate */
-	mdelay(10);
-	if (context->ctrl_urb) {
-		usb_unlink_urb(context->ctrl_urb);
-		usb_free_urb(context->ctrl_urb);
-		context->ctrl_urb = NULL;
-	}
+	usb_kill_urb(context->ctrl_urb);
+	usb_free_urb(context->ctrl_urb);
+	context->ctrl_urb = NULL;
 	hisax_unregister(&context->d_if);
 	kfree(context);		/* free our structure again */
 }				/* hfc_usb_disconnect */
drivers/isdn/hisax/st5481_usb.c +0 -1
Index: usb-2.6/drivers/isdn/hisax/st5481_usb.c
===================================================================
@@ -407,7 +407,6 @@ fill_isoc_urb(struct urb *urb, struct us
 {
 	int k;
 
-	spin_lock_init(&urb->lock);
 	urb->dev=dev;
 	urb->pipe=pipe;
 	urb->interval = 1;
#1 lore raw

This email was totally broken.

We do sign-offs and ack's per patch.

We do one patch per email.

And we don't do attachments.

Please don't send patches like this. There's ample documentation on how to send patches properly, there's no excuse for not bothering to do it right.

Linus