Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1030250Ab2JDIwJ (ORCPT ); Thu, 4 Oct 2012 04:52:09 -0400 Received: from ams-iport-1.cisco.com ([144.254.224.140]:54558 "EHLO ams-iport-1.cisco.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S965535Ab2JDIwH (ORCPT ); Thu, 4 Oct 2012 04:52:07 -0400 X-IronPort-AV: E=Sophos;i="4.80,533,1344211200"; d="scan'208";a="144756028" From: Hans Verkuil To: Prabhakar Subject: Re: [PATCH] media: davinci: vpif: Add return code check at vb2_queue_init() Date: Thu, 4 Oct 2012 10:51:51 +0200 User-Agent: KMail/1.13.7 (Linux/3.5-0.slh.1-aptosid-amd64; KDE/4.8.4; x86_64; ; ) Cc: DLOS , LMML , Manjunath Hadli , VGER , "Lad, Prabhakar" , Hans Verkuil References: <1349271749-24426-1-git-send-email-prabhakar.lad@ti.com> In-Reply-To: <1349271749-24426-1-git-send-email-prabhakar.lad@ti.com> MIME-Version: 1.0 Content-Type: Text/Plain; charset="utf-8" Content-Transfer-Encoding: 7bit Message-Id: <201210041051.51656.hverkuil@xs4all.nl> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2963 Lines: 87 Hi Prabhakar, I think this introduces a memory leak, see my comments below. On Wed 3 October 2012 15:42:29 Prabhakar wrote: > From: Lad, Prabhakar > > from commit with id 896f38f582730a19eb49677105b4fe4c0270b82e > it's mandatory to check the return code of vb2_queue_init(). > > Signed-off-by: Lad, Prabhakar > Signed-off-by: Manjunath Hadli > Cc: Hans Verkuil > --- > drivers/media/platform/davinci/vpif_capture.c | 8 ++++++-- > drivers/media/platform/davinci/vpif_display.c | 8 ++++++-- > 2 files changed, 12 insertions(+), 4 deletions(-) > > diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c > index 83b80ba..3521725 100644 > --- a/drivers/media/platform/davinci/vpif_capture.c > +++ b/drivers/media/platform/davinci/vpif_capture.c > @@ -976,6 +976,7 @@ static int vpif_reqbufs(struct file *file, void *priv, > struct common_obj *common; > u8 index = 0; > struct vb2_queue *q; > + int ret; > > vpif_dbg(2, debug, "vpif_reqbufs\n"); > > @@ -1015,8 +1016,11 @@ static int vpif_reqbufs(struct file *file, void *priv, > q->mem_ops = &vb2_dma_contig_memops; > q->buf_struct_size = sizeof(struct vpif_cap_buffer); > > - vb2_queue_init(q); > - > + ret = vb2_queue_init(q); > + if (ret) { > + vpif_err("vpif_capture: vb2_queue_init() failed\n"); I think you need to call vb2_dma_contig_cleanup_ctx(common->alloc_ctx); here to prevent a memory leak. > + return ret; > + } > /* Set io allowed member of file handle to TRUE */ > fh->io_allowed[index] = 1; > /* Increment io usrs member of channel object to 1 */ > diff --git a/drivers/media/platform/davinci/vpif_display.c b/drivers/media/platform/davinci/vpif_display.c > index ae8329d..84cee9d 100644 > --- a/drivers/media/platform/davinci/vpif_display.c > +++ b/drivers/media/platform/davinci/vpif_display.c > @@ -936,6 +936,7 @@ static int vpif_reqbufs(struct file *file, void *priv, > enum v4l2_field field; > struct vb2_queue *q; > u8 index = 0; > + int ret; > > /* This file handle has not initialized the channel, > It is not allowed to do settings */ > @@ -981,8 +982,11 @@ static int vpif_reqbufs(struct file *file, void *priv, > q->mem_ops = &vb2_dma_contig_memops; > q->buf_struct_size = sizeof(struct vpif_disp_buffer); > > - vb2_queue_init(q); > - > + ret = vb2_queue_init(q); > + if (ret) { > + vpif_err("vpif_display: vb2_queue_init() failed\n"); Ditto. > + return ret; > + } > /* Set io allowed member of file handle to TRUE */ > fh->io_allowed[index] = 1; > /* Increment io usrs member of channel object to 1 */ > Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/