This patch fixes a memory leak by freeing priv in mc13783_regulator_remove
Signed-off-by: Axel Lin <[email protected]>
Cc: Sascha Hauer <[email protected]>
Cc: Liam Girdwood <[email protected]>
Cc: Mark Brown <[email protected]>
Cc: Samuel Ortiz <[email protected]>
---
drivers/regulator/mc13783-regulator.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/regulator/mc13783-regulator.c
b/drivers/regulator/mc13783-regulator.c
index a681f5e..ad036dd 100644
--- a/drivers/regulator/mc13783-regulator.c
+++ b/drivers/regulator/mc13783-regulator.c
@@ -618,9 +618,12 @@ static int __devexit
mc13783_regulator_remove(struct platform_device *pdev)
dev_get_platdata(&pdev->dev);
int i;
+ platform_set_drvdata(pdev, NULL);
+
for (i = 0; i < pdata->num_regulators; i++)
regulator_unregister(priv->regulators[i]);
+ kfree(priv);
return 0;
}
--
1.5.4.3
On Mon, 2010-04-19 at 09:58 +0800, Axel Lin wrote:
> This patch fixes a memory leak by freeing priv in mc13783_regulator_remove
>
> Signed-off-by: Axel Lin <[email protected]>
> Cc: Sascha Hauer <[email protected]>
> Cc: Liam Girdwood <[email protected]>
> Cc: Mark Brown <[email protected]>
> Cc: Samuel Ortiz <[email protected]>
> ---
> drivers/regulator/mc13783-regulator.c | 3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
>
Applied.
Thanks
Liam
--
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk
On Mon, Apr 19, 2010 at 09:58:02AM +0800, Axel Lin wrote:
> This patch fixes a memory leak by freeing priv in mc13783_regulator_remove
Acked-by: Mark Brown <[email protected]>
but note that...
> + platform_set_drvdata(pdev, NULL);
> +
This is completely unrelated to what your description says (and is not
needed).
hi Mark,
2010/4/20 Mark Brown <[email protected]>:
> On Mon, Apr 19, 2010 at 09:58:02AM +0800, Axel Lin wrote:
>> This patch fixes a memory leak by freeing priv in mc13783_regulator_remove
>
> Acked-by: Mark Brown <[email protected]>
>
> but note that...
>
>> + ? ? ? platform_set_drvdata(pdev, NULL);
>> +
>
> This is completely unrelated to what your description says (and is not
> needed).
>
In the probe function , the driver uses platform_set_drvdata(pdev,
priv) to store a pointer to the priv data structure.
To avoid leaving a dangling pointer behind, the driver should clear
the pointer to priv before freeing priv.
Regards,
Axel
On Tue, Apr 20, 2010 at 01:34:18PM +0800, Axel Lin wrote:
> 2010/4/20 Mark Brown <[email protected]>:
> > On Mon, Apr 19, 2010 at 09:58:02AM +0800, Axel Lin wrote:
> >> + ? ? ? platform_set_drvdata(pdev, NULL);
> >> +
> > This is completely unrelated to what your description says (and is not
> > needed).
> In the probe function , the driver uses platform_set_drvdata(pdev,
> priv) to store a pointer to the priv data structure.
> To avoid leaving a dangling pointer behind, the driver should clear
> the pointer to priv before freeing priv.
All of which is totally unrelated to the description of the patch. One
of the things that I do when I'm reviewing is look to see if the patch
does what the description says - unrelated changes are normally a red
flag that something is wrong and there are mistakes or unintended side
effects lurking in the code.
The dangling pointer isn't really a problem in any case; if a driver
is relying on the behaviour of the pointer between bindings it's in
trouble anyway since there aren't any guarantees about what happens.
See the recent discussion about the same issue for I2C.