USB: Fix error cleanup path in airprime

Fix up the error processing path: in usb_submit_urb failed, we forgot
to free buffers. Also, don't free buffers in read callback: less error
prone, 21 LOC less, no need to comment so much. N.B. write path is ok
to do kfree.

Signed-off-by: Pete Zaitcev <[email protected]>

---

Dunno what I was raving and ranting about, I guess I was really ill.
The driver is solid except for the unnecessarily convoluted kfree-ing.
No oopses, only a small leak.

diff --git a/drivers/usb/serial/airprime.c b/drivers/usb/serial/airprime.c
index f2ca76a..0839a2d 100644
--- a/drivers/usb/serial/airprime.c
+++ b/drivers/usb/serial/airprime.c
@@ -58,11 +58,6 @@ static void airprime_read_bulk_callback(struct urb *urb)
 	if (urb->status) {
 		dbg("%s - nonzero read bulk status received: %d",
 		    __FUNCTION__, urb->status);
-		/* something happened, so free up the memory for this urb */
-		if (urb->transfer_buffer) {
-			kfree (urb->transfer_buffer);
-			urb->transfer_buffer = NULL;
-		}
 		return;
 	}
 	usb_serial_debug_data(debug, &port->dev, __FUNCTION__, urb->actual_length, data);
@@ -146,6 +141,8 @@ static int airprime_open(struct usb_serial_port *port, struct file *filp)
 				  airprime_read_bulk_callback, port);
 		result = usb_submit_urb(urb, GFP_KERNEL);
 		if (result) {
+			usb_free_urb(urb);
+			kfree(buffer);
 			dev_err(&port->dev,
 				"%s - failed submitting read urb %d for port %d, error %d\n",
 				__FUNCTION__, i, port->number, result);
@@ -160,27 +157,12 @@ static int airprime_open(struct usb_serial_port *port, struct file *filp)
 	/* some error happened, cancel any submitted urbs and clean up anything that
 	   got allocated successfully */
 
-	for ( ; i >= 0; --i) {
+	while (i-- != 0) {
 		urb = priv->read_urbp[i];
-		if (urb) {
-			/* This urb was submitted successfully. So we have to
-			   cancel it.
-			   Unlinking the urb will invoke read_bulk_callback()
-			   with an error status, so its transfer buffer will
-			   be freed there */
-			if (usb_unlink_urb (urb) != -EINPROGRESS) {
-				/* comments in drivers/usb/core/urb.c say this
-				   can only happen if the urb was never submitted,
-				   or has completed already.
-				   Either way we may have to free the transfer
-				   buffer here. */
-				if (urb->transfer_buffer) {
-					kfree (urb->transfer_buffer);
-					urb->transfer_buffer = NULL;
-				}
-			}
-			usb_free_urb (urb);
-		}
+		buffer = urb->transfer_buffer;
+		usb_kill_urb (urb);
+		usb_free_urb (urb);
+		kfree (buffer);
 	}
 
  out:
@@ -194,10 +176,9 @@ static void airprime_close(struct usb_serial_port *port, struct file * filp)
 
 	dbg("%s - port %d", __FUNCTION__, port->number);
 
-	/* killing the urb will invoke read_bulk_callback() with an error status,
-	   so the transfer buffer will be freed there */
 	for (i = 0; i < NUM_READ_URBS; ++i) {
 		usb_kill_urb (priv->read_urbp[i]);
+		kfree (priv->read_urbp[i]->transfer_buffer);
 		usb_free_urb (priv->read_urbp[i]);
 	}
 

-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier.
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel