From: Mark Brown <broonie@opensource.wolfsonmicro.com>
To: Axel Lin <axel.lin@gmail.com>
Cc: linux-kernel <linux-kernel@vger.kernel.org>,
Sascha Hauer <s.hauer@pengutronix.de>,
Liam Girdwood <lrg@slimlogic.co.uk>,
Samuel Ortiz <sameo@linux.intel.com>
Subject: Re: [PATCH] mc13783-regulator: fix a memory leak in mc13783_regulator_remove
Date: Wed, 21 Apr 2010 01:32:42 +0900 [thread overview]
Message-ID: <20100420163241.GA30625@opensource.wolfsonmicro.com> (raw)
In-Reply-To: <s2y427d64721004192234x96d286e4teab6f32323d2b291@mail.gmail.com>
On Tue, Apr 20, 2010 at 01:34:18PM +0800, Axel Lin wrote:
> 2010/4/20 Mark Brown <broonie@opensource.wolfsonmicro.com>:
> > 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.
prev parent reply other threads:[~2010-04-20 16:32 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-04-19 1:58 [PATCH] mc13783-regulator: fix a memory leak in mc13783_regulator_remove Axel Lin
2010-04-19 12:34 ` Liam Girdwood
2010-04-19 17:01 ` Mark Brown
2010-04-20 5:34 ` Axel Lin
2010-04-20 16:32 ` Mark Brown [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20100420163241.GA30625@opensource.wolfsonmicro.com \
--to=broonie@opensource.wolfsonmicro.com \
--cc=axel.lin@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=lrg@slimlogic.co.uk \
--cc=s.hauer@pengutronix.de \
--cc=sameo@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox