On Wed, 18 Dec 2002 11:57:48 +0100
Oliver Neukum <[EMAIL PROTECTED]> wrote:
> Am Mittwoch, 18. Dezember 2002 01:46 schrieb Kari Hameenaho:
> > On Wed, 18 Dec 2002 01:18:19 +0100
> >
> > "Oliver Neukum" <[EMAIL PROTECTED]> wrote:
> > > > So here cleanup will use NULL pointer for sure, it set set to NULL in
> > > > other path and tested to be NULL in the other.
> > >
> > > Both take usblp->sem, don't they?
> >
> > But does locking matter when setting a pointer to NULL and then using
> > that NULL pointer as parameter to usb_bugger_free(). So this is not a
> > problem of locking, but setting a pointer to NULL and then using the
> > pointer later (called from same function in usblp_disconnect() or if
> > device was open, then called later in usb_release() function).
> >
> > I cannot understand why usblp->sem has anything to do with this, the
> > code is sequential ?
>
> You are right, there's no connection at all.
> Currently there's a memory leak on unplugging a printer.
> Well, how about saving the usb_device twice in the device descriptor
> and use the second save only to free the buffers?
>
Here is the most minimal patch with that approach.
------------ 2.5.52_usblp_keep_dev.patch ---------
--- linux-2.5.52/drivers/usb/class/usblp.c Mon Dec 16 04:07:48 2002
+++ linux-2.5.52-usb/drivers/usb/class/usblp.c Wed Dec 18 22:37:13 2002
@@ -130,6 +130,7 @@
struct usblp {
struct usb_device *dev; /* USB device */
+ struct usb_device *cleanup_dev; /* same as above, cleanup only
+*/
devfs_handle_t devfs; /* devfs device */
struct semaphore sem; /* locks this struct,
especially "dev" */
char *writebuf; /* write transfer_buffer */
@@ -388,9 +389,9 @@
usb_deregister_dev (1, usblp->minor);
info("usblp%d: removed", usblp->minor);
- usb_buffer_free (usblp->dev, USBLP_BUF_SIZE,
+ usb_buffer_free (usblp->cleanup_dev, USBLP_BUF_SIZE,
usblp->writebuf, usblp->writeurb->transfer_dma);
- usb_buffer_free (usblp->dev, USBLP_BUF_SIZE,
+ usb_buffer_free (usblp->cleanup_dev, USBLP_BUF_SIZE,
usblp->readbuf, usblp->writeurb->transfer_dma);
kfree (usblp->device_id_string);
kfree (usblp->statusbuf);
@@ -828,7 +829,7 @@
goto abort;
}
memset(usblp, 0, sizeof(struct usblp));
- usblp->dev = dev;
+ usblp->cleanup_dev = usblp->dev = dev;
init_MUTEX (&usblp->sem);
init_waitqueue_head(&usblp->wait);
usblp->ifnum = intf->altsetting->desc.bInterfaceNumber;
It survives at least some disconnects with no problem and is quite obvious
otherwise too.
---
Kari H�meenaho
-------------------------------------------------------
This SF.NET email is sponsored by: Order your Holiday Geek Presents Now!
Green Lasers, Hip Geek T-Shirts, Remote Control Tanks, Caffeinated Soap,
MP3 Players, XBox Games, Flying Saucers, WebCams, Smart Putty.
T H I N K G E E K . C O M http://www.thinkgeek.com/sf/
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
