Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751818AbbL1JUY (ORCPT ); Mon, 28 Dec 2015 04:20:24 -0500 Received: from mail2-relais-roc.national.inria.fr ([192.134.164.83]:54776 "EHLO mail2-relais-roc.national.inria.fr" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750975AbbL1JUR (ORCPT ); Mon, 28 Dec 2015 04:20:17 -0500 X-IronPort-AV: E=Sophos;i="5.20,490,1444687200"; d="scan'208";a="194760484" Date: Mon, 28 Dec 2015 10:20:10 +0100 (CET) From: Julia Lawall X-X-Sender: jll@hadrien To: SF Markus Elfring cc: linux-media@vger.kernel.org, Mauro Carvalho Chehab , LKML , kernel-janitors@vger.kernel.org Subject: Re: [PATCH] [media] tuners: One check less in m88rs6000t_get_rf_strength() after error detection In-Reply-To: <5680FDB3.7060305@users.sourceforge.net> Message-ID: References: <566ABCD9.1060404@users.sourceforge.net> <5680FDB3.7060305@users.sourceforge.net> User-Agent: Alpine 2.10 (DEB 1266 2009-07-14) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2620 Lines: 87 On Mon, 28 Dec 2015, SF Markus Elfring wrote: > From: Markus Elfring > Date: Mon, 28 Dec 2015 10:10:34 +0100 > > This issue was detected by using the Coccinelle software. > > Move the jump label directly before the desired log statement > so that the variable "ret" will not be checked once more > after it was determined that a function call failed. Why not avoid both unnecessary ifs and the enormous ugliness of a label inside an if by making two returns: a return 0 for success and a dev_dbg and return ret for failure? julia > Signed-off-by: Markus Elfring > --- > drivers/media/tuners/m88rs6000t.c | 16 +++++++++------- > 1 file changed, 9 insertions(+), 7 deletions(-) > > diff --git a/drivers/media/tuners/m88rs6000t.c b/drivers/media/tuners/m88rs6000t.c > index 504bfbc..b45594e 100644 > --- a/drivers/media/tuners/m88rs6000t.c > +++ b/drivers/media/tuners/m88rs6000t.c > @@ -510,27 +510,27 @@ static int m88rs6000t_get_rf_strength(struct dvb_frontend *fe, u16 *strength) > > ret = regmap_read(dev->regmap, 0x5A, &val); > if (ret) > - goto err; > + goto report_failure; > RF_GC = val & 0x0f; > > ret = regmap_read(dev->regmap, 0x5F, &val); > if (ret) > - goto err; > + goto report_failure; > IF_GC = val & 0x0f; > > ret = regmap_read(dev->regmap, 0x3F, &val); > if (ret) > - goto err; > + goto report_failure; > TIA_GC = (val >> 4) & 0x07; > > ret = regmap_read(dev->regmap, 0x77, &val); > if (ret) > - goto err; > + goto report_failure; > BB_GC = (val >> 4) & 0x0f; > > ret = regmap_read(dev->regmap, 0x76, &val); > if (ret) > - goto err; > + goto report_failure; > PGA2_GC = val & 0x3f; > PGA2_cri = PGA2_GC >> 2; > PGA2_crf = PGA2_GC & 0x03; > @@ -562,9 +562,11 @@ static int m88rs6000t_get_rf_strength(struct dvb_frontend *fe, u16 *strength) > /* scale value to 0x0000-0xffff */ > gain = clamp_val(gain, 1000U, 10500U); > *strength = (10500 - gain) * 0xffff / (10500 - 1000); > -err: > - if (ret) > + > + if (ret) { > +report_failure: > dev_dbg(&dev->client->dev, "failed=%d\n", ret); > + } > return ret; > } > > -- > 2.6.3 > > -- > 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 > -- 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/