Message ID | Pine.LNX.4.64.1008052229390.31692@ask.diku.dk (mailing list archive) |
---|---|
State | Superseded, archived |
Headers |
Return-path: <linux-media-owner@vger.kernel.org> Envelope-to: mchehab@infradead.org Delivery-date: Thu, 05 Aug 2010 20:31:00 +0000 Received: from bombadil.infradead.org [18.85.46.34] by pedra with IMAP (fetchmail-6.3.17) for <mchehab@localhost> (single-drop); Thu, 05 Aug 2010 17:36:00 -0300 (BRT) Received: from vger.kernel.org ([209.132.180.67]) by bombadil.infradead.org with esmtp (Exim 4.72 #1 (Red Hat Linux)) id 1Oh75g-0001de-MN; Thu, 05 Aug 2010 20:31:00 +0000 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934411Ab0HEUaB (ORCPT <rfc822;kmpark@infradead.org> + 1 other); Thu, 5 Aug 2010 16:30:01 -0400 Received: from mgw2.diku.dk ([130.225.96.92]:39125 "EHLO mgw2.diku.dk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934404Ab0HEU35 (ORCPT <rfc822;linux-media@vger.kernel.org>); Thu, 5 Aug 2010 16:29:57 -0400 Received: from localhost (localhost [127.0.0.1]) by mgw2.diku.dk (Postfix) with ESMTP id C01CF19BF16; Thu, 5 Aug 2010 22:29:56 +0200 (CEST) Received: from mgw2.diku.dk ([127.0.0.1]) by localhost (mgw2.diku.dk [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 30467-12; Thu, 5 Aug 2010 22:29:55 +0200 (CEST) Received: from nhugin.diku.dk (nhugin.diku.dk [130.225.96.140]) by mgw2.diku.dk (Postfix) with ESMTP id 7B41119BF15; Thu, 5 Aug 2010 22:29:55 +0200 (CEST) Received: from ask.diku.dk (ask.diku.dk [130.225.96.225]) by nhugin.diku.dk (Postfix) with ESMTP id 1DA6D6DF893; Thu, 5 Aug 2010 22:28:46 +0200 (CEST) Received: by ask.diku.dk (Postfix, from userid 3767) id 60C23200C3; Thu, 5 Aug 2010 22:29:55 +0200 (CEST) Received: from localhost (localhost [127.0.0.1]) by ask.diku.dk (Postfix) with ESMTP id 5434B200BD; Thu, 5 Aug 2010 22:29:55 +0200 (CEST) Date: Thu, 5 Aug 2010 22:29:55 +0200 (CEST) From: Julia Lawall <julia@diku.dk> To: Mauro Carvalho Chehab <mchehab@infradead.org>, linux-media@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org Subject: [PATCH 42/42] drivers/media/video/bt8xx: Adjust confusing if indentation Message-ID: <Pine.LNX.4.64.1008052229390.31692@ask.diku.dk> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: <linux-media.vger.kernel.org> X-Mailing-List: linux-media@vger.kernel.org |
Commit Message
Julia Lawall
Aug. 5, 2010, 8:29 p.m. UTC
From: Julia Lawall <julia@diku.dk> Indent the branch of an if. The semantic match that finds this problem is as follows: (http://coccinelle.lip6.fr/) // <smpl> @r disable braces4@ position p1,p2; statement S1,S2; @@ ( if (...) { ... } | if (...) S1@p1 S2@p2 ) @script:python@ p1 << r.p1; p2 << r.p2; @@ if (p1[0].column == p2[0].column): cocci.print_main("branch",p1) cocci.print_secs("after",p2) // </smpl> Signed-off-by: Julia Lawall <julia@diku.dk> --- drivers/media/video/bt8xx/bttv-i2c.c | 2 +- 1 file changed, 1 insertion(+), 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
Hi, one minor comment: On Thu, Aug 5, 2010 at 10:29 PM, Julia Lawall <julia@diku.dk> wrote: > From: Julia Lawall <julia@diku.dk> > > Indent the branch of an if. [...] > diff --git a/drivers/media/video/bt8xx/bttv-i2c.c b/drivers/media/video/bt8xx/bttv-i2c.c > index 685d659..695765c 100644 > --- a/drivers/media/video/bt8xx/bttv-i2c.c > +++ b/drivers/media/video/bt8xx/bttv-i2c.c > @@ -123,7 +123,7 @@ bttv_i2c_wait_done(struct bttv *btv) > if (wait_event_interruptible_timeout(btv->i2c_queue, > btv->i2c_done, msecs_to_jiffies(85)) == -ERESTARTSYS) > > - rc = -EIO; > + rc = -EIO; I'd also remove the empty line before the indented statement, it's confusing... Luca -- 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 Thursday 05 August 2010 22:51:12 Luca Tettamanti wrote: > > diff --git a/drivers/media/video/bt8xx/bttv-i2c.c b/drivers/media/video/bt8xx/bttv-i2c.c > > index 685d659..695765c 100644 > > --- a/drivers/media/video/bt8xx/bttv-i2c.c > > +++ b/drivers/media/video/bt8xx/bttv-i2c.c > > @@ -123,7 +123,7 @@ bttv_i2c_wait_done(struct bttv *btv) > > if (wait_event_interruptible_timeout(btv->i2c_queue, > > btv->i2c_done, msecs_to_jiffies(85)) == -ERESTARTSYS) > > > > - rc = -EIO; > > + rc = -EIO; > > I'd also remove the empty line before the indented statement, it's confusing... > The entire function looks a bit weird to me. If you look at the caller, you'll notice that -EIO is treated in the same way as if the function had returned zero, so the entire if() clause is pointless (the wait_event_* probably is not). Moreover, returning -ERESTARTSYS is probably the right action here, why else would you make the wait interruptible? Arnd -- 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
diff --git a/drivers/media/video/bt8xx/bttv-i2c.c b/drivers/media/video/bt8xx/bttv-i2c.c index 685d659..695765c 100644 --- a/drivers/media/video/bt8xx/bttv-i2c.c +++ b/drivers/media/video/bt8xx/bttv-i2c.c @@ -123,7 +123,7 @@ bttv_i2c_wait_done(struct bttv *btv) if (wait_event_interruptible_timeout(btv->i2c_queue, btv->i2c_done, msecs_to_jiffies(85)) == -ERESTARTSYS) - rc = -EIO; + rc = -EIO; if (btv->i2c_done & BT848_INT_RACK) rc = 1;