On Mon, 1 Jun 2020 at 13:37, Ben Skeggs <skeggsb@xxxxxxxxx> wrote: > > On Mon, 1 Jun 2020 at 13:27, <dinghao.liu@xxxxxxxxxx> 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 > > > returns a pointer, the core will call the destructor to clean things > > > up. > > > > > > > I'm not familiar with the behavior of the caller of gm20b_clk_new(). > > If the subdev constructor fails, the core will check the pointer > > (here is "pclk"), then it's ok and there is no bug (Do you mean > > this?). If the core executes error handling code only according to > > the error code, there may be a memory leak bug (the caller cannot > > know if -ENOMEM comes from the failure of kzalloc or gk20a_clk_ctor). > > If the core always calls the destructor as long as the constructor > > fails (even if the kzalloc fails), we may have a double free bug. > > > > Would you like to give a more detailed explanation about the behavior > > of the core? > 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 have a bug here if kzalloc() fails as it doesn't overwrite the previous pointer from gm20b_clk_new(). That whole ctor() sequence is written a little strangely. > > Ben. > > > > Regards, > > Dinghao _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
