Messages in this thread | | | Date | Wed, 7 May 2014 13:38:58 +0900 | Subject | Re: [PATCH] [media] s2255drv: fix memory leak s2255_probe() | From | DaeSeok Youn <> |
| |
Hello,
2014-05-05 17:38 GMT+09:00 Sakari Ailus <sakari.ailus@iki.fi>: > On Tue, Apr 15, 2014 at 07:54:43PM +0900, DaeSeok Youn wrote: >> Hi, Sakari >> >> 2014-04-15 18:33 GMT+09:00 Sakari Ailus <sakari.ailus@iki.fi>: >> > Hi Daeseok, >> > >> > On Tue, Apr 15, 2014 at 01:49:34PM +0900, Daeseok Youn wrote: >> >> >> >> smatch says: >> >> drivers/media/usb/s2255/s2255drv.c:2246 s2255_probe() warn: >> >> possible memory leak of 'dev' >> >> >> >> Signed-off-by: Daeseok Youn <daeseok.youn@gmail.com> >> >> --- >> >> drivers/media/usb/s2255/s2255drv.c | 1 + >> >> 1 files changed, 1 insertions(+), 0 deletions(-) >> >> >> >> diff --git a/drivers/media/usb/s2255/s2255drv.c b/drivers/media/usb/s2255/s2255drv.c >> >> index 1d4ba2b..8aca3ef 100644 >> >> --- a/drivers/media/usb/s2255/s2255drv.c >> >> +++ b/drivers/media/usb/s2255/s2255drv.c >> >> @@ -2243,6 +2243,7 @@ static int s2255_probe(struct usb_interface *interface, >> >> dev->cmdbuf = kzalloc(S2255_CMDBUF_SIZE, GFP_KERNEL); >> >> if (dev->cmdbuf == NULL) { >> >> s2255_dev_err(&interface->dev, "out of memory\n"); >> >> + kfree(dev); >> >> return -ENOMEM; >> >> } >> >> >> > >> > The rest of the function already uses goto and labels for error handling. I >> > think it'd take adding one more. dev is correctly released in other error >> > cases. >> I am not sure that adding a new label for error handling when >> allocation for dev->cmdbuf is failed. >> I think it is ok to me. :-) Because I think it is not good adding a >> new label and use goto statement for this. > > I can ack this if you use the same pattern for error handling that's already > there. But it has a redundant kfree() call for dev->cmdbuf when it failed to allocate, right? If it is ok, I send this again after fixing as your comment.
Thanks. Daeseok Youn. > > -- > Sakari Ailus > e-mail: sakari.ailus@iki.fi XMPP: sailus@retiisi.org.uk
| |