Dan Carpenter
2020-Jun-02 15:39 UTC
[Nouveau] [PATCH] drm/nouveau/clk/gm20b: Fix memory leak in gm20b_clk_new()
On Tue, Jun 02, 2020 at 01:10:34PM +0200, Markus Elfring wrote:> > The original patch was basically fine. > > I propose to reconsider the interpretation of the software situation once more. > > * Should the allocated clock object be kept usable even after > a successful return from this function?Heh. You're right. The patch is freeing "clk" on the success path so that doesn't work. regards, dan carpenter
dinghao.liu at zju.edu.cn
2020-Jun-03 02:21 UTC
[Nouveau] [PATCH] drm/nouveau/clk/gm20b: Fix memory leak in gm20b_clk_new()
> On Tue, Jun 02, 2020 at 01:10:34PM +0200, Markus Elfring wrote: > > > The original patch was basically fine. > > > > I propose to reconsider the interpretation of the software situation once more. > > > > * Should the allocated clock object be kept usable even after > > a successful return from this function? > > Heh. You're right. The patch is freeing "clk" on the success path so > that doesn't work. >Ben has explained this problem: https://lore.kernel.org/patchwork/patch/1249592/ Since the caller will check "pclk" on failure, we don't need to free "clk" in gm20b_clk_new() and I think this patch is no longer needed. Regards, Dinghao
Markus Elfring
2020-Jun-03 05:04 UTC
[Nouveau] drm/nouveau/clk/gm20b: Understanding challenges around gm20b_clk_new()
> Ben has explained this problem: > https://lore.kernel.org/patchwork/patch/1249592/ > Since the caller will check "pclk" on failure, we don't need to free > "clk" in gm20b_clk_new() and I think this patch is no longer needed.* I am curious if it can become easier to see the relationships for these variables according to mentioned ?destructor? calls. * Did you notice opportunities to improve source code analysis (or software documentation) accordingly? Regards, Markus
Possibly Parallel Threads
- [PATCH] drm/nouveau/clk/gm20b: Fix memory leak in gm20b_clk_new()
- [PATCH] drm/nouveau/clk/gm20b: Fix memory leak in gm20b_clk_new
- [PATCH] drm/nouveau/clk/gm20b: Fix memory leak in gm20b_clk_new
- drm/nouveau/clk/gm20b: Fix memory leak in gm20b_clk_new()
- [PATCH] drm/nouveau/clk/gm20b: Fix memory leak in gm20b_clk_new