Re: Re: [PATCH] drm/nouveau/clk/gm20b: Fix memory leak in gm20b_clk_new()

2020-06-03 Thread dinghao . liu
> 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

Re: [PATCH] drm/nouveau/clk/gm20b: Fix memory leak in gm20b_clk_new()

2020-06-03 Thread Markus Elfring
> 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? * How much do “destructor” calls matter here for (sub)devices? >

Re: [PATCH] drm/nouveau/clk/gm20b: Fix memory leak in gm20b_clk_new()

2020-06-02 Thread 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

Re: Re: [PATCH] drm/nouveau/clk/gm20b: Fix memory leak in gm20b_clk_new()

2020-06-02 Thread Dan Carpenter
The original patch was basically fine. Just add a Fixes tag and resend. regards, dan carpenter ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Re: [PATCH] drm/nouveau/clk/gm20b: Fix memory leak in gm20b_clk_new()

2020-06-02 Thread dinghao . liu
> > > It's the same when gm20b_clk_new() returns from elsewhere following this > > call. > > I suggest to reconsider the interpretation of the software situation once > more. > Can it be that the allocated clock object should be kept usable even after > a successful return from this function?

Re: [PATCH] drm/nouveau/clk/gm20b: Fix memory leak in gm20b_clk_new()

2020-06-02 Thread Markus Elfring
> When gk20a_clk_ctor() returns an error code, pointer "clk" > should be released. Such an information is reasonable. > It's the same when gm20b_clk_new() returns from elsewhere following this call. I suggest to reconsider the interpretation of the software situation once more. Can it be that

Re: Re: Re: [PATCH] drm/nouveau/clk/gm20b: Fix memory leak in gm20b_clk_new

2020-06-02 Thread dinghao . liu
> > If there's *any* error, it'll check the pointer, if it's non-NULL, > > it'll call the destructor. If kzalloc() fails, the pointer will be > > NULL, there's no double-free bug. *every* subdev is written this way > > to avoid duplicating cleanup logic. > Actually, gm20b_clk_new_speedo0() may

Re: Re: [PATCH] drm/nouveau/clk/gm20b: Fix memory leak in gm20b_clk_new

2020-06-02 Thread dinghao . liu
Hi Ben, > > When gk20a_clk_ctor() returns an error code, pointer "clk" > > should be released. It's the same when gm20b_clk_new() > > returns from elsewhere following this call. > This shouldn't be necessary. If a subdev constructor fails, and > returns a pointer, the core will call the

Re: Re: [PATCH] drm/nouveau/clk/gm20b: Fix memory leak in gm20b_clk_new

2020-05-31 Thread Ben Skeggs
On Mon, 1 Jun 2020 at 13:37, Ben Skeggs wrote: > > On Mon, 1 Jun 2020 at 13:27, wrote: > > > > > > Hi Ben, > > > > > > When gk20a_clk_ctor() returns an error code, pointer "clk" > > > > should be released. It's the same when gm20b_clk_new() > > > > returns from elsewhere following this call. > >

Re: Re: [PATCH] drm/nouveau/clk/gm20b: Fix memory leak in gm20b_clk_new

2020-05-31 Thread Ben Skeggs
On Mon, 1 Jun 2020 at 13:27, wrote: > > > Hi Ben, > > > > When gk20a_clk_ctor() returns an error code, pointer "clk" > > > should be released. It's the same when gm20b_clk_new() > > > returns from elsewhere following this call. > > This shouldn't be necessary. If a subdev constructor fails, and

Re: [PATCH] drm/nouveau/clk/gm20b: Fix memory leak in gm20b_clk_new

2020-05-31 Thread Ben Skeggs
On Sat, 30 May 2020 at 19:42, Dinghao Liu wrote: > > When gk20a_clk_ctor() returns an error code, pointer "clk" > should be released. It's the same when gm20b_clk_new() > returns from elsewhere following this call. This shouldn't be necessary. If a subdev constructor fails, and returns a