[1/2] omap3: sr: fix memory leak and simplify the code

Commit Message

Artem Bityutskiy July 9, 2010, 1:20 p.m. UTC

From: Artem Bityutskiy <[e-mail]>

This patch fixes the following problem indicated by kmemleak:

kmemleak: unreferenced object 0xdf93c280 (size 64):
kmemleak:   backtrace:
kmemleak:     [<c00df6d4>] create_object+0x104/0x200
kmemleak:     [<c00d7638>] kmem_cache_alloc+0xe4/0xf4
kmemleak:     [<c000fc38>] omap_devinit_smartreflex+0x44/0x244
kmemleak:     [<c003a33c>] do_one_initcall+0x5c/0x1b8
kmemleak:     [<c00083fc>] kernel_init+0x94/0x110
kmemleak:     [<c003ba04>] kernel_thread_exit+0x0/0x8

The reason is that 'omap_devinit_smartreflex()' allocates 'sr_data',
then passes it to 'omap_device_build()', which 'kmemdup()'s it and
uses the copy. But 'omap_devinit_smartreflex()' never frees 'sr_data'.

This patch make 'sr_data' to be a stack variable, which eliminates
the memory leak and simplifies the code a bit.

Signed-off-by: Artem Bityutskiy <[e-mail]>
---
 arch/arm/mach-omap2/sr_device.c |   27 +++++++++------------------
 1 files changed, 9 insertions(+), 18 deletions(-)

Comments

Artem Bityutskiy July 9, 2010, 1:22 p.m. UTC | #1

On Fri, 2010-07-09 at 16:20 +0300, Artem Bityutskiy wrote:
> From: Artem Bityutskiy <[e-mail]>
> 
> This patch fixes the following problem indicated by kmemleak:
> 
> kmemleak: unreferenced object 0xdf93c280 (size 64):
> kmemleak:   backtrace:
> kmemleak:     [<c00df6d4>] create_object+0x104/0x200
> kmemleak:     [<c00d7638>] kmem_cache_alloc+0xe4/0xf4
> kmemleak:     [<c000fc38>] omap_devinit_smartreflex+0x44/0x244
> kmemleak:     [<c003a33c>] do_one_initcall+0x5c/0x1b8
> kmemleak:     [<c00083fc>] kernel_init+0x94/0x110
> kmemleak:     [<c003ba04>] kernel_thread_exit+0x0/0x8
> 
> The reason is that 'omap_devinit_smartreflex()' allocates 'sr_data',
> then passes it to 'omap_device_build()', which 'kmemdup()'s it and
> uses the copy. But 'omap_devinit_smartreflex()' never frees 'sr_data'.
> 
> This patch make 'sr_data' to be a stack variable, which eliminates
> the memory leak and simplifies the code a bit.
> 
> Signed-off-by: Artem Bityutskiy <[e-mail]>
> ---
>  arch/arm/mach-omap2/sr_device.c |   27 +++++++++------------------
>  1 files changed, 9 insertions(+), 18 deletions(-)

Note, I only compile-tested both of the patches, so be careful.

Patch

diff --git a/arch/arm/mach-omap2/sr_device.c b/arch/arm/mach-omap2/sr_device.c
index 24630ef..d1bb495 100644
--- a/arch/arm/mach-omap2/sr_device.c
+++ b/arch/arm/mach-omap2/sr_device.c
@@ -129,7 +129,7 @@  static int __init omap_devinit_smartreflex(void)
 	char *name = "smartreflex";
 
 	do {
-		struct omap_smartreflex_data *sr_data;
+		struct omap_smartreflex_data sr_data;
 		struct omap_smartreflex_dev_data *sr_dev_data;
 		struct omap_device *od;
 		struct omap_hwmod *oh;
@@ -140,15 +140,7 @@  static int __init omap_devinit_smartreflex(void)
 		if (!oh)
 			break;
 
-		sr_data = kzalloc(sizeof(struct omap_smartreflex_data),
-								GFP_KERNEL);
 		sr_dev_data = (struct omap_smartreflex_dev_data *)oh->dev_attr;
-		if (!sr_data) {
-			pr_warning("%s: could not find hwmod data for %d\n",
-					__func__, i + 1);
-			kfree(sr_data);
-			return -ENOMEM;
-		}
 
 		/*
 		 * OMAP3430 ES3.1 chips by default come with Efuse burnt
@@ -159,26 +151,25 @@  static int __init omap_devinit_smartreflex(void)
 		 */
 		if (cpu_is_omap343x()) {
 			if (omap_rev() == OMAP3430_REV_ES3_1)
-				sr_data->enable_on_init = true;
+				sr_data.enable_on_init = true;
 			else
-				sr_data->enable_on_init = false;
+				sr_data.enable_on_init = false;
 		} else {
-			sr_data->enable_on_init = false;
+			sr_data.enable_on_init = false;
 		}
-		sr_data->device_enable = omap_device_enable;
-		sr_data->device_shutdown = omap_device_shutdown;
-		sr_data->device_idle = omap_device_idle;
+		sr_data.device_enable = omap_device_enable;
+		sr_data.device_shutdown = omap_device_shutdown;
+		sr_data.device_idle = omap_device_idle;
 		omap_get_voltage_table(i, &sr_dev_data->volt_data,
 					&sr_dev_data->volts_supported);
-		sr_set_nvalues(sr_dev_data, sr_data);
+		sr_set_nvalues(sr_dev_data, &sr_data);
 
-		od = omap_device_build(name, i, oh, sr_data, sizeof(*sr_data),
+		od = omap_device_build(name, i, oh, &sr_data, sizeof(sr_data),
 				       omap_sr_latency,
 				       ARRAY_SIZE(omap_sr_latency), 0);
 		if (IS_ERR(od)) {
 			pr_warning("%s: Could not build omap_device %s:%s\n",
 				__func__, name, oh->name);
-			kfree(sr_data);
 		}
 		i++;
 	} while (1);