Message ID | CAPgLHd8UFD4p=PAK+Ukno8qvmvaxVxvSrrZw=qpUtERCyP7hpg@mail.gmail.com (mailing list archive) |
---|---|
State | Changes Requested, archived |
Delegated to: | Hans Verkuil |
Headers |
Received: from mail.tu-berlin.de ([130.149.7.33]) by www.linuxtv.org with esmtp (Exim 4.72) (envelope-from <linux-media-owner@vger.kernel.org>) id 1Ublnb-0006OE-7z; Mon, 13 May 2013 07:59:51 +0200 X-tubIT-Incoming-IP: 209.132.180.67 Received: from vger.kernel.org ([209.132.180.67]) by mail.tu-berlin.de (exim-4.72/mailfrontend-8) with esmtp id 1UblnZ-00084S-kM; Mon, 13 May 2013 07:59:51 +0200 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753429Ab3EMF7r (ORCPT <rfc822;mkrufky@linuxtv.org> + 1 other); Mon, 13 May 2013 01:59:47 -0400 Received: from mail-bk0-f53.google.com ([209.85.214.53]:41040 "EHLO mail-bk0-f53.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753115Ab3EMF7r (ORCPT <rfc822;linux-media@vger.kernel.org>); Mon, 13 May 2013 01:59:47 -0400 Received: by mail-bk0-f53.google.com with SMTP id mx1so14606bkb.40 for <linux-media@vger.kernel.org>; Sun, 12 May 2013 22:59:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:x-received:date:message-id:subject:from:to:cc :content-type; bh=vFaNi1yLWnuhOMYMZVp8iNxB7Qtkmux088Cg2g1YjZU=; b=jiOyeJgItCA5RxA7L8xay/AW8cy7B3C+94Y6KZtB56Ra37x8BeYZovvdddyRRfEYA4 DaE5YcnGhYUT+6dPl+0rhuDwJxOt/6HeuOKZO+m7naEQOk6XPmbezJfka0V7s86Agufs dnsEa9MK++NOHUmC9S5Mscn1XXuXLv2rbMH6Jto/XwymaayD5Gq8FcGs163wRA+1+NsI Q1QahkFaoTel0+0+bdmmx+Ud8Dfl84cEtfsNOje0q2m62XJaOYeje66NPqvy0XU/E0Lx sOOR7Ic5U7shT3ymmrFjK0Mg6VFY/AzqbaO3d9ofT8lEZKCTCpZIlArzJvc01VPRM4U5 ZqFw== MIME-Version: 1.0 X-Received: by 10.204.57.13 with SMTP id a13mr5214040bkh.63.1368424785760; Sun, 12 May 2013 22:59:45 -0700 (PDT) Received: by 10.204.199.129 with HTTP; Sun, 12 May 2013 22:59:45 -0700 (PDT) Date: Mon, 13 May 2013 13:59:45 +0800 Message-ID: <CAPgLHd8UFD4p=PAK+Ukno8qvmvaxVxvSrrZw=qpUtERCyP7hpg@mail.gmail.com> Subject: [PATCH] [media] sta2x11_vip: fix error return code in sta2x11_vip_init_one() From: Wei Yongjun <weiyj.lk@gmail.com> To: mchehab@redhat.com, hans.verkuil@cisco.com, giancarlo.asnaghi@st.com, federico.vaga@gmail.com, prabhakar.csengg@gmail.com Cc: yongjun_wei@trendmicro.com.cn, linux-media@vger.kernel.org Content-Type: text/plain; charset=ISO-8859-1 Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: <linux-media.vger.kernel.org> X-Mailing-List: linux-media@vger.kernel.org X-PMX-Version: 6.0.0.2142326, Antispam-Engine: 2.7.2.2107409, Antispam-Data: 2013.5.13.54816 X-PMX-Spam: Gauge=IIIIIIIII, Probability=9%, Report=' CN_TLD 0.1, FORGED_FROM_GMAIL 0.1, MULTIPLE_RCPTS 0.1, HTML_00_01 0.05, HTML_00_10 0.05, BODYTEXTP_SIZE_3000_LESS 0, BODY_SIZE_1000_1099 0, BODY_SIZE_2000_LESS 0, BODY_SIZE_5000_LESS 0, BODY_SIZE_7000_LESS 0, DKIM_SIGNATURE 0, URI_ENDS_IN_HTML 0, WEBMAIL_SOURCE 0, __ANY_URI 0, __CP_URI_IN_BODY 0, __CT 0, __CT_TEXT_PLAIN 0, __DATE_TZ_HK 0, __FRAUD_WEBMAIL 0, __FRAUD_WEBMAIL_FROM 0, __FROM_GMAIL 0, __HAS_FROM 0, __HAS_MSGID 0, __HAS_X_MAILING_LIST 0, __MIME_TEXT_ONLY 0, __MIME_VERSION 0, __MULTIPLE_RCPTS_CC_X2 0, __MULTIPLE_RCPTS_TO_X5 0, __PHISH_SPEAR_HTTP_RECEIVED 0, __PHISH_SPEAR_STRUCTURE_1 0, __PHISH_SPEAR_STRUCTURE_2 0, __SANE_MSGID 0, __TO_MALFORMED_2 0, __TO_NO_NAME 0, __URI_NO_WWW 0, __URI_NS , __YOUTUBE_RCVD 0' |
Commit Message
Wei Yongjun
May 13, 2013, 5:59 a.m. UTC
From: Wei Yongjun <yongjun_wei@trendmicro.com.cn> Fix to return a negative error code from the error handling case instead of 0, as done elsewhere in this function. Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn> --- drivers/media/pci/sta2x11/sta2x11_vip.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Comments
Hello, I agree with the content of the patch, but I disagree with the commit message. From the commit message it seems that you fixed a bug about the error code, but the aim of this patch is to uniform the code style. I suggest something like: "uniform code style in sta2x11_vip_init_one()" On Monday 13 May 2013 13:59:45 Wei Yongjun wrote: > From: Wei Yongjun <yongjun_wei@trendmicro.com.cn> > > Fix to return a negative error code from the error handling > case instead of 0, as done elsewhere in this function. > > Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn> > --- > drivers/media/pci/sta2x11/sta2x11_vip.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/pci/sta2x11/sta2x11_vip.c > b/drivers/media/pci/sta2x11/sta2x11_vip.c index 7005695..77edc11 100644 > --- a/drivers/media/pci/sta2x11/sta2x11_vip.c > +++ b/drivers/media/pci/sta2x11/sta2x11_vip.c > @@ -1047,7 +1047,8 @@ static int sta2x11_vip_init_one(struct pci_dev *pdev, > ret = sta2x11_vip_init_controls(vip); > if (ret) > goto free_mem; > - if (v4l2_device_register(&pdev->dev, &vip->v4l2_dev)) > + ret = v4l2_device_register(&pdev->dev, &vip->v4l2_dev); > + if (ret) > goto free_mem; > > dev_dbg(&pdev->dev, "BAR #0 at 0x%lx 0x%lx irq %d\n",
On 05/13/2013 08:19 PM, Federico Vaga wrote: > Hello, > > I agree with the content of the patch, but I disagree with the commit message. > >From the commit message it seems that you fixed a bug about the error code, > but the aim of this patch is to uniform the code style. I suggest something > like: "uniform code style in sta2x11_vip_init_one()" The orig code will release all the resources if v4l2_device_register() failed and return 0. But what we need in this case is to return an negative error code to let the caller known we are failed. So the patch save the return value of v4l2_device_register() to 'ret' and return it when error. > > On Monday 13 May 2013 13:59:45 Wei Yongjun wrote: >> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn> >> >> Fix to return a negative error code from the error handling >> case instead of 0, as done elsewhere in this function. >> >> Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn> >> --- >> drivers/media/pci/sta2x11/sta2x11_vip.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/media/pci/sta2x11/sta2x11_vip.c >> b/drivers/media/pci/sta2x11/sta2x11_vip.c index 7005695..77edc11 100644 >> --- a/drivers/media/pci/sta2x11/sta2x11_vip.c >> +++ b/drivers/media/pci/sta2x11/sta2x11_vip.c >> @@ -1047,7 +1047,8 @@ static int sta2x11_vip_init_one(struct pci_dev *pdev, >> ret = sta2x11_vip_init_controls(vip); >> if (ret) >> goto free_mem; >> - if (v4l2_device_register(&pdev->dev, &vip->v4l2_dev)) >> + ret = v4l2_device_register(&pdev->dev, &vip->v4l2_dev); >> + if (ret) >> goto free_mem; >> >> dev_dbg(&pdev->dev, "BAR #0 at 0x%lx 0x%lx irq %d\n", -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Monday 13 May 2013 20:40:33 Wei Yongjun wrote: > On 05/13/2013 08:19 PM, Federico Vaga wrote: > > Hello, > > > > I agree with the content of the patch, but I disagree with the commit > > message.> > > >From the commit message it seems that you fixed a bug about the error > > >code, > > > > but the aim of this patch is to uniform the code style. I suggest > > something > > like: "uniform code style in sta2x11_vip_init_one()" > > The orig code will release all the resources if v4l2_device_register() > failed and return 0. > But what we need in this case is to return an negative error code > to let the caller known we are failed. So the patch save the return > value of v4l2_device_register() to 'ret' and return it when error. Oh sure, you are right :) I did not understand it immediately from the commit message. Can you put this answer as commit message? It is perfectly clear where is the bug now. Thank you
diff --git a/drivers/media/pci/sta2x11/sta2x11_vip.c b/drivers/media/pci/sta2x11/sta2x11_vip.c index 7005695..77edc11 100644 --- a/drivers/media/pci/sta2x11/sta2x11_vip.c +++ b/drivers/media/pci/sta2x11/sta2x11_vip.c @@ -1047,7 +1047,8 @@ static int sta2x11_vip_init_one(struct pci_dev *pdev, ret = sta2x11_vip_init_controls(vip); if (ret) goto free_mem; - if (v4l2_device_register(&pdev->dev, &vip->v4l2_dev)) + ret = v4l2_device_register(&pdev->dev, &vip->v4l2_dev); + if (ret) goto free_mem; dev_dbg(&pdev->dev, "BAR #0 at 0x%lx 0x%lx irq %d\n",