Nouveau - Jun 2020 - [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

> 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

> 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