From fc7db0182076df7a325640f236aae663ddf2ccca Mon Sep 17 00:00:00 2001 From: patacongo Date: Tue, 26 Jul 2011 17:27:57 +0000 Subject: [PATCH] Correct USB mass storage memory leak git-svn-id: svn://svn.code.sf.net/p/nuttx/code/trunk@3820 42af7a65-404d-4744-a932-0658087f49c3 --- ChangeLog | 9 ++++++++- TODO | 25 +------------------------ arch/arm/src/stm32/stm32_usbdev.c | 11 +++++++++-- drivers/usbdev/usbdev_storage.c | 17 +++++++++-------- 4 files changed, 27 insertions(+), 35 deletions(-) diff --git a/ChangeLog b/ChangeLog index 396f3ae88b4..3cafb16103d 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1944,4 +1944,11 @@ Distribution Format (BDF) into fonts that can be used in the NX graphics system. * include/nuttx/nx: Move all NX header files from include/nuttx to - include/nuttx/nx. \ No newline at end of file + include/nuttx/nx. + * drivers/usbdev/usbdev_usbstorage.c and arch/arm/src/stm32/stm32_usbdev.c: + Correct a memory leak when the USB mass storage driver is connected and + then disconnected from the target. The write requests were not being + freed. NOTE that the unregister logic in the STM32 needed to call + stm32_reset() in order to return the write requests to the USB mass + storage driver; it is possible that a similar but could exist for other + architectures. diff --git a/TODO b/TODO index f9fc8614ab7..d4f34a1dfd5 100644 --- a/TODO +++ b/TODO @@ -42,7 +42,7 @@ apps/ (5) Network Utilities (apps/netutils/) (5) NuttShell (NSH) (apps/nshlib) - (6) Other Applications & Tests (apps/examples/) + (5) Other Applications & Tests (apps/examples/) o Task/Scheduler (sched/) ^^^^^^^^^^^^^^^^^^^^^^^ @@ -1318,26 +1318,3 @@ o Other Applications & Tests (apps/examples/) the artifact is larger. Status: Open Priority: Medium. - - Description: USB Mass Storage Memory Leak. When examples/usbstorage is - built as an NuttShell (NSH) built-in, there appears to be a - memory leak that occurs when a USB mass storage connection is - established and then torn down. This leak is 608 bytes each - time: - - NuttShell (NSH) NuttX-6.6 - nsh> free - total used free largest - Mem: 59360 9536 49824 49824 - nsh> msconn - total used free largest - Mem: 59360 15296 44064 41712 - nsh> msdis - nsh> free - total used free largest - Mem: 59360 10144 49216 41712 <-- Leaked 608 bytes - Status: Open - Priority: Medium. Obviously, this must be fixed. However the USB - Mass storage example is fine for supporting most development - needs even thought it does leak. - diff --git a/arch/arm/src/stm32/stm32_usbdev.c b/arch/arm/src/stm32/stm32_usbdev.c index 3fe54ea1775..c1d6da803d1 100644 --- a/arch/arm/src/stm32/stm32_usbdev.c +++ b/arch/arm/src/stm32/stm32_usbdev.c @@ -3622,9 +3622,15 @@ int usbdev_unregister(struct usbdevclass_driver_s *driver) } #endif - /* Unbind the class driver */ + /* Reset the hardware and cancel all requests. All requests must be + * canceled while the class driver is still bound. + */ flags = irqsave(); + stm32_reset(priv); + + /* Unbind the class driver */ + CLASS_UNBIND(driver, &priv->usbdev); /* Disable USB controller interrupts (but keep them attached) */ @@ -3633,7 +3639,8 @@ int usbdev_unregister(struct usbdevclass_driver_s *driver) up_disable_irq(STM32_IRQ_USBLPCANRX0); /* Put the hardware in an inactive state. Then bring the hardware back up - * in the reset state. + * in the reset state (this is probably not necessary, the stm32_reset() + * call above was probably sufficient). */ stm32_hwshutdown(priv); diff --git a/drivers/usbdev/usbdev_storage.c b/drivers/usbdev/usbdev_storage.c index 4f2ffcc6e7a..029cd748e94 100644 --- a/drivers/usbdev/usbdev_storage.c +++ b/drivers/usbdev/usbdev_storage.c @@ -670,14 +670,6 @@ static void usbstrg_unbind(FAR struct usbdev_s *dev) usbstrg_resetconfig(priv); up_mdelay(50); - /* Free the bulk IN endpoint */ - - if (priv->epbulkin) - { - DEV_FREEEP(dev, priv->epbulkin); - priv->epbulkin = NULL; - } - /* Free the pre-allocated control request */ if (priv->ctrlreq != NULL) @@ -721,6 +713,15 @@ static void usbstrg_unbind(FAR struct usbdev_s *dev) usbstrg_freereq(priv->epbulkin, reqcontainer->req); } } + + /* Free the bulk IN endpoint */ + + if (priv->epbulkin) + { + DEV_FREEEP(dev, priv->epbulkin); + priv->epbulkin = NULL; + } + irqrestore(flags); } }