qcserial: fix a memory leak in qcprobe error path

In current implemtation, the "data" is not kfreed in qcprobe error path. This patch moves the memory allocation a little bit latter and only allocate memory when no error is detected in previous checking.

drivers/usb/serial/qcserial.c +1 -1

 	}
 
-	return retval;
+out:

Quoted 3 lines

In current implemtation, the "data" is not kfreed in qcprobe error path. This patch moves the memory allocation a little bit latter and only allocate memory when no error is detected in previous checking.

Yeah, but it's now pretty messy.

drivers/usb/serial/qcserial.c static int qcprobe(struct usb_serial *serial, const struct usb_device_id *id) · line 118

Quoted 13 lines

drivers/usb/serial/qcserial.c +0 -7

@@ -109,13 +109,6 @@ static int qcprobe(struct usb_serial *serial, const struct usb_device_id *id)
       ifnum = intf->desc.bInterfaceNumber;
       dbg("This Interface = %d", ifnum);
 
-     data = serial->private = kzalloc(sizeof(struct usb_wwan_intf_private),
-                                      GFP_KERNEL);
-     if (!data)
-             return -ENOMEM;
-
-     spin_lock_init(&data->susp_lock);
-

Moving this to the end is fine.

drivers/usb/serial/qcserial.c static int qcprobe(struct usb_serial *serial, const struct usb_device_id *id) · line 138

Quoted 6 lines

drivers/usb/serial/qcserial.c +1 -1

@@ -140,7 +135,7 @@ static int qcprobe(struct usb_serial *serial, const struct usb_device_id *id)
                                       retval);
                               retval = -ENODEV;
                       }
-                     return retval;
+                     goto out;

But this doesn't need to be changed, right?
Or this.

drivers/usb/serial/qcserial.c static int qcprobe(struct usb_serial *serial, const struct usb_device_id *id) · line 154

Quoted 9 lines

drivers/usb/serial/qcserial.c +1 -1

               }
               break;
 

@@ -156,17 +151,26 @@ static int qcprobe(struct usb_serial *serial, const struct usb_device_id *id)
                                       retval);
                               retval = -ENODEV;
                       }
-                     return retval;
+                     goto out;

Or this?

Quoted 7 lines

drivers/usb/serial/qcserial.c +0 -0

               }
               break;
 

      default:
              dev_err(&serial->dev->dev,
                      "unknown number of interfaces: %d\n", nintf);
-             return -ENODEV;

Hm. maybe.

Quoted 4 lines

      }

-     return retval;

drivers/usb/serial/qcserial.c +1 -0

+out:

drivers/usb/serial/qcserial.c static int qcprobe(struct usb_serial *serial, const struct usb_device_id *id) · line 173

Quoted 10 lines

drivers/usb/serial/qcserial.c +10 -0

+     if (retval)
+             return retval;
+
+     data = serial->private = kzalloc(sizeof(struct usb_wwan_intf_private),
+                                      GFP_KERNEL);
+     if (!data)
+             return -ENOMEM;
+
+     spin_lock_init(&data->susp_lock);
+     return 0;

I guess it's ok, I just don't like doing data initialization on the "out" path, it's messy.

Care to neaten this up a bit more and resend it?  Perhaps just free the memory on the error path instead of moving it later on?

thanks,

greg k-h