Message ID | 20170219103412.10092-1-christophe.jaillet@wanadoo.fr (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Hi Christophe, Am 19.02.2017 um 11:34 schrieb Christophe JAILLET: > If 'kzalloc()' fails, we should release resources allocated so far, just as > done in all other cases in this function. > > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> > --- > Not sure that the error handling path is correct. > Is 'gdev[0]' freed? Should it be? sorry, didn't checked your patch yet. Currently there are 3 bcm2835 drivers in staging (vchiq, camera, audio). So please resend with a more distinct subject. Thanks Stefan > --- > drivers/staging/media/platform/bcm2835/bcm2835-camera.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/staging/media/platform/bcm2835/bcm2835-camera.c b/drivers/staging/media/platform/bcm2835/bcm2835-camera.c > index ca15a698e018..9651b9bc3439 100644 > --- a/drivers/staging/media/platform/bcm2835/bcm2835-camera.c > +++ b/drivers/staging/media/platform/bcm2835/bcm2835-camera.c > @@ -1914,8 +1914,10 @@ static int __init bm2835_mmal_init(void) > > for (camera = 0; camera < num_cameras; camera++) { > dev = kzalloc(sizeof(struct bm2835_mmal_dev), GFP_KERNEL); > - if (!dev) > - return -ENOMEM; > + if (!dev) { > + ret = -ENOMEM; > + goto free_dev; > + } > > dev->camera_num = camera; > dev->max_width = resolutions[camera][0];
On Fri, Feb 24, 2017 at 01:37:30PM +0100, Stefan Wahren wrote: > Hi Christophe, > > Am 19.02.2017 um 11:34 schrieb Christophe JAILLET: > >If 'kzalloc()' fails, we should release resources allocated so far, just as > >done in all other cases in this function. > > > >Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> > >--- > >Not sure that the error handling path is correct. > >Is 'gdev[0]' freed? Should it be? > Yes, but I already sent a patch to fix this and your leak as well and Greg merged it. > sorry, didn't checked your patch yet. It takes like 30 seconds to review this patch. Do you use mutt? I have a macro that applies patches and loads vim at the right line. regards, dan carpenter
> Dan Carpenter <dan.carpenter@oracle.com> hat am 24. Februar 2017 um 20:57 geschrieben: > > > On Fri, Feb 24, 2017 at 01:37:30PM +0100, Stefan Wahren wrote: > > Hi Christophe, > > > > Am 19.02.2017 um 11:34 schrieb Christophe JAILLET: > > >If 'kzalloc()' fails, we should release resources allocated so far, just as > > >done in all other cases in this function. > > > > > >Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> > > >--- > > >Not sure that the error handling path is correct. > > >Is 'gdev[0]' freed? Should it be? > > > > Yes, but I already sent a patch to fix this and your leak as well and > Greg merged it. My leak? I'm confused.
On Fri, Feb 24, 2017 at 10:38:38PM +0100, Stefan Wahren wrote: > > > Dan Carpenter <dan.carpenter@oracle.com> hat am 24. Februar 2017 um 20:57 geschrieben: > > > > > > On Fri, Feb 24, 2017 at 01:37:30PM +0100, Stefan Wahren wrote: > > > Hi Christophe, > > > > > > Am 19.02.2017 um 11:34 schrieb Christophe JAILLET: > > > >If 'kzalloc()' fails, we should release resources allocated so far, just as > > > >done in all other cases in this function. > > > > > > > >Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> > > > >--- > > > >Not sure that the error handling path is correct. > > > >Is 'gdev[0]' freed? Should it be? > > > > > > > Yes, but I already sent a patch to fix this and your leak as well and > > Greg merged it. > > My leak? I'm confused. The one you're fixing I mean.
diff --git a/drivers/staging/media/platform/bcm2835/bcm2835-camera.c b/drivers/staging/media/platform/bcm2835/bcm2835-camera.c index ca15a698e018..9651b9bc3439 100644 --- a/drivers/staging/media/platform/bcm2835/bcm2835-camera.c +++ b/drivers/staging/media/platform/bcm2835/bcm2835-camera.c @@ -1914,8 +1914,10 @@ static int __init bm2835_mmal_init(void) for (camera = 0; camera < num_cameras; camera++) { dev = kzalloc(sizeof(struct bm2835_mmal_dev), GFP_KERNEL); - if (!dev) - return -ENOMEM; + if (!dev) { + ret = -ENOMEM; + goto free_dev; + } dev->camera_num = camera; dev->max_width = resolutions[camera][0];
If 'kzalloc()' fails, we should release resources allocated so far, just as done in all other cases in this function. Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> --- Not sure that the error handling path is correct. Is 'gdev[0]' freed? Should it be? --- drivers/staging/media/platform/bcm2835/bcm2835-camera.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)