From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751353AbeBTIIA (ORCPT ); Tue, 20 Feb 2018 03:08:00 -0500 Received: from mail-wm0-f68.google.com ([74.125.82.68]:55849 "EHLO mail-wm0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751172AbeBTIH5 (ORCPT ); Tue, 20 Feb 2018 03:07:57 -0500 X-Google-Smtp-Source: AH8x22453H0h6IA4RnXcW31+SdnNs3R0fFMoovNtsgV96P8jwSmhIQ4UvYUlt/3s/Z/V3cSd2f+SEQ== Subject: Re: [PATCH v2] [media] Use common error handling code in 20 functions To: SF Markus Elfring , Sean Young , Seung-Woo Kim , Shyam Saini , Thomas Gleixner , Wei Yongjun Cc: linux-media@vger.kernel.org, Al Viro , Andi Shyti , Andrew Morton , Andrey Utkin , Arvind Yadav , Bhumika Goyal , Bjorn Helgaas , Brian Johnson , =?UTF-8?Q?Christoph_B=c3=b6hmwalder?= , Christophe Jaillet , Colin Ian King , Daniele Nicolodi , =?UTF-8?Q?David_H=c3=a4rdeman?= , Devendra Sharma , "Gustavo A. R. Silva" , Hans Verkuil , Inki Dae , Joe Perches , Kees Cook , Laurent Pinchart , Masahiro Yamada , Mauro Carvalho Chehab , Max Kellermann , Mike Isely , Philippe Ombredanne , Sakari Ailus , Santosh Kumar Singh , Satendra Singh Thakur , LKML , kernel-janitors@vger.kernel.org References: <227d2d7c-5aee-1190-1624-26596a048d9c@users.sourceforge.net> From: Todor Tomov Message-ID: Date: Tue, 20 Feb 2018 10:07:52 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 MIME-Version: 1.0 In-Reply-To: <227d2d7c-5aee-1190-1624-26596a048d9c@users.sourceforge.net> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Markus, Thank you for the patch. On 19.02.2018 20:11, SF Markus Elfring wrote: > From: Markus Elfring > Date: Mon, 19 Feb 2018 18:50:40 +0100 > > Adjust jump targets so that a bit of exception handling can be better > reused at the end of these functions. > > This issue was partly detected by using the Coccinelle software. > > Signed-off-by: Markus Elfring > --- > > v2: > Hans Verkuil insisted on patch squashing. Thus several changes > were recombined based on source files from Linux next-20180216. > > The implementation of the function "tda8261_set_params" was improved > after a notification by Christoph Böhmwalder on 2017-09-26. > > drivers/media/dvb-core/dmxdev.c | 16 ++++---- > drivers/media/dvb-frontends/tda1004x.c | 20 ++++++---- > drivers/media/dvb-frontends/tda8261.c | 19 ++++++---- > drivers/media/pci/bt8xx/dst.c | 19 ++++++---- > drivers/media/pci/bt8xx/dst_ca.c | 30 +++++++-------- > drivers/media/pci/cx88/cx88-input.c | 17 +++++---- > drivers/media/platform/omap3isp/ispvideo.c | 29 +++++++-------- > .../media/platform/qcom/camss-8x16/camss-csid.c | 20 +++++----- > drivers/media/tuners/tuner-xc2028.c | 30 +++++++-------- > drivers/media/usb/cpia2/cpia2_usb.c | 13 ++++--- > drivers/media/usb/gspca/gspca.c | 17 +++++---- > drivers/media/usb/gspca/sn9c20x.c | 17 +++++---- > drivers/media/usb/pvrusb2/pvrusb2-ioread.c | 10 +++-- > drivers/media/usb/tm6000/tm6000-cards.c | 7 ++-- > drivers/media/usb/tm6000/tm6000-dvb.c | 11 ++++-- > drivers/media/usb/tm6000/tm6000-video.c | 13 ++++--- > drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c | 13 +++---- > drivers/media/usb/ttusb-dec/ttusb_dec.c | 43 ++++++++-------------- > drivers/media/usb/uvc/uvc_v4l2.c | 13 ++++--- > 19 files changed, 180 insertions(+), 177 deletions(-) > > diff --git a/drivers/media/dvb-core/dmxdev.c b/drivers/media/dvb-core/dmxdev.c > index 6d53af00190e..6a0411c91195 100644 > diff --git a/drivers/media/platform/qcom/camss-8x16/camss-csid.c b/drivers/media/platform/qcom/camss-8x16/camss-csid.c > index 64df82817de3..92d4dc6b4a66 100644 > --- a/drivers/media/platform/qcom/camss-8x16/camss-csid.c > +++ b/drivers/media/platform/qcom/camss-8x16/camss-csid.c > @@ -328,16 +328,12 @@ static int csid_set_power(struct v4l2_subdev *sd, int on) > return ret; > > ret = csid_set_clock_rates(csid); > - if (ret < 0) { > - regulator_disable(csid->vdda); > - return ret; > - } > + if (ret < 0) > + goto disable_regulator; > > ret = camss_enable_clocks(csid->nclocks, csid->clock, dev); > - if (ret < 0) { > - regulator_disable(csid->vdda); > - return ret; > - } > + if (ret < 0) > + goto disable_regulator; > > enable_irq(csid->irq); > > @@ -345,8 +341,7 @@ static int csid_set_power(struct v4l2_subdev *sd, int on) > if (ret < 0) { > disable_irq(csid->irq); > camss_disable_clocks(csid->nclocks, csid->clock); > - regulator_disable(csid->vdda); > - return ret; > + goto disable_regulator; > } > > hw_version = readl_relaxed(csid->base + CAMSS_CSID_HW_VERSION); > @@ -357,6 +352,11 @@ static int csid_set_power(struct v4l2_subdev *sd, int on) > ret = regulator_disable(csid->vdda); > } > > + goto exit; I think it will be cleaner if you remove the exit label and return here instead. > + > +disable_regulator: > + regulator_disable(csid->vdda); > +exit: > return ret; > } -- Best regards, Todor Tomov From mboxrd@z Thu Jan 1 00:00:00 1970 From: Todor Tomov Date: Tue, 20 Feb 2018 08:07:52 +0000 Subject: Re: [PATCH v2] [media] Use common error handling code in 20 functions Message-Id: List-Id: References: <227d2d7c-5aee-1190-1624-26596a048d9c@users.sourceforge.net> In-Reply-To: <227d2d7c-5aee-1190-1624-26596a048d9c@users.sourceforge.net> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable To: SF Markus Elfring , Sean Young , Seung-Woo Kim , Shyam Saini , Thomas Gleixner , Wei Yongjun Cc: linux-media@vger.kernel.org, Al Viro , Andi Shyti , Andrew Morton , Andrey Utkin , Arvind Yadav , Bhumika Goyal , Bjorn Helgaas , Brian Johnson , =?UTF-8?Q?Christoph_B=c3=b6hmwalder?= , Christophe Jaillet , Colin Ian King , Daniele Nicolodi , =?UTF-8?Q?David_H=c3=a4rdeman?= , Devendra Sharma , "Gustavo A. R. Silva" , Hans Verkuil , Inki Dae , Joe Perches , Kees Cook , Laurent Pinchart , Masahiro Yamada , Mauro Carvalho Chehab , Max Kellermann , Mike Isely , Philippe Ombredanne , Sakari Ailus , Santosh Kumar Singh , Satendra Singh Thakur , LKML , kernel-janitors@vger.kernel.org Hi Markus, Thank you for the patch. On 19.02.2018 20:11, SF Markus Elfring wrote: > From: Markus Elfring > Date: Mon, 19 Feb 2018 18:50:40 +0100 >=20 > Adjust jump targets so that a bit of exception handling can be better > reused at the end of these functions. >=20 > This issue was partly detected by using the Coccinelle software. >=20 > Signed-off-by: Markus Elfring > --- >=20 > v2: > Hans Verkuil insisted on patch squashing. Thus several changes > were recombined based on source files from Linux next-20180216. >=20 > The implementation of the function "tda8261_set_params" was improved > after a notification by Christoph B=C3=B6hmwalder on 2017-09-26. >=20 > drivers/media/dvb-core/dmxdev.c | 16 ++++---- > drivers/media/dvb-frontends/tda1004x.c | 20 ++++++---- > drivers/media/dvb-frontends/tda8261.c | 19 ++++++---- > drivers/media/pci/bt8xx/dst.c | 19 ++++++---- > drivers/media/pci/bt8xx/dst_ca.c | 30 +++++++-------- > drivers/media/pci/cx88/cx88-input.c | 17 +++++---- > drivers/media/platform/omap3isp/ispvideo.c | 29 +++++++-------- > .../media/platform/qcom/camss-8x16/camss-csid.c | 20 +++++----- > drivers/media/tuners/tuner-xc2028.c | 30 +++++++-------- > drivers/media/usb/cpia2/cpia2_usb.c | 13 ++++--- > drivers/media/usb/gspca/gspca.c | 17 +++++---- > drivers/media/usb/gspca/sn9c20x.c | 17 +++++---- > drivers/media/usb/pvrusb2/pvrusb2-ioread.c | 10 +++-- > drivers/media/usb/tm6000/tm6000-cards.c | 7 ++-- > drivers/media/usb/tm6000/tm6000-dvb.c | 11 ++++-- > drivers/media/usb/tm6000/tm6000-video.c | 13 ++++--- > drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c | 13 +++---- > drivers/media/usb/ttusb-dec/ttusb_dec.c | 43 ++++++++--------= ------ > drivers/media/usb/uvc/uvc_v4l2.c | 13 ++++--- > 19 files changed, 180 insertions(+), 177 deletions(-) >=20 > diff --git a/drivers/media/dvb-core/dmxdev.c b/drivers/media/dvb-core/dmx= dev.c > index 6d53af00190e..6a0411c91195 100644 > diff --git a/drivers/media/platform/qcom/camss-8x16/camss-csid.c b/driver= s/media/platform/qcom/camss-8x16/camss-csid.c > index 64df82817de3..92d4dc6b4a66 100644 > --- a/drivers/media/platform/qcom/camss-8x16/camss-csid.c > +++ b/drivers/media/platform/qcom/camss-8x16/camss-csid.c > @@ -328,16 +328,12 @@ static int csid_set_power(struct v4l2_subdev *sd, i= nt on) > return ret; > =20 > ret =3D csid_set_clock_rates(csid); > - if (ret < 0) { > - regulator_disable(csid->vdda); > - return ret; > - } > + if (ret < 0) > + goto disable_regulator; > =20 > ret =3D camss_enable_clocks(csid->nclocks, csid->clock, dev); > - if (ret < 0) { > - regulator_disable(csid->vdda); > - return ret; > - } > + if (ret < 0) > + goto disable_regulator; > =20 > enable_irq(csid->irq); > =20 > @@ -345,8 +341,7 @@ static int csid_set_power(struct v4l2_subdev *sd, int= on) > if (ret < 0) { > disable_irq(csid->irq); > camss_disable_clocks(csid->nclocks, csid->clock); > - regulator_disable(csid->vdda); > - return ret; > + goto disable_regulator; > } > =20 > hw_version =3D readl_relaxed(csid->base + CAMSS_CSID_HW_VERSION); > @@ -357,6 +352,11 @@ static int csid_set_power(struct v4l2_subdev *sd, in= t on) > ret =3D regulator_disable(csid->vdda); > } > =20 > + goto exit; I think it will be cleaner if you remove the exit label and return here ins= tead. > + > +disable_regulator: > + regulator_disable(csid->vdda); > +exit: > return ret; > } --=20 Best regards, Todor Tomov -- To unsubscribe from this list: send the line "unsubscribe kernel-janitors" = in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html