From patchwork Tue Nov 20 19:10:35 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eilon Greenstein X-Patchwork-Id: 200497 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id B5D4B2C007A for ; Wed, 21 Nov 2012 06:11:30 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752355Ab2KTTLQ (ORCPT ); Tue, 20 Nov 2012 14:11:16 -0500 Received: from mms1.broadcom.com ([216.31.210.17]:4705 "EHLO mms1.broadcom.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751582Ab2KTTLP (ORCPT ); Tue, 20 Nov 2012 14:11:15 -0500 Received: from [10.9.200.133] by mms1.broadcom.com with ESMTP (Broadcom SMTP Relay (Email Firewall v6.5)); Tue, 20 Nov 2012 11:09:18 -0800 X-Server-Uuid: 06151B78-6688-425E-9DE2-57CB27892261 Received: from mail-irva-13.broadcom.com (10.11.16.103) by IRVEXCHHUB02.corp.ad.broadcom.com (10.9.200.133) with Microsoft SMTP Server id 8.2.247.2; Tue, 20 Nov 2012 11:10:45 -0800 Received: from [10.185.6.73] (lb-tlvb-eilong.il.broadcom.com [10.185.6.73]) by mail-irva-13.broadcom.com (Postfix) with ESMTP id 738C740FE4; Tue, 20 Nov 2012 11:11:07 -0800 (PST) Message-ID: <1353438635.10779.10.camel@lb-tlvb-eilong.il.broadcom.com> Subject: Re: [PATCH v2] checkpatch: add double empty line check From: "Eilon Greenstein" Reply-to: eilong@broadcom.com To: "Andy Whitcroft" cc: "Joe Perches" , "David Rientjes" , "linux-kernel@vger.kernel.org" , netdev Date: Tue, 20 Nov 2012 21:10:35 +0200 In-Reply-To: <20121120163607.GB17797@dm> References: <1353151057.14327.18.camel@lb-tlvb-eilong.il.broadcom.com> <20121120115239.GA7955@dm> <1353421624.6559.9.camel@lb-tlvb-eilong.il.broadcom.com> <20121120144329.GE7955@dm> <1353424027.6559.15.camel@lb-tlvb-eilong.il.broadcom.com> <20121120154443.GK7955@dm> <1353427570.6559.21.camel@lb-tlvb-eilong.il.broadcom.com> <20121120161417.GA17797@dm> <20121120163607.GB17797@dm> Organization: Broadcom X-Mailer: Evolution 3.2.3-0ubuntu6 MIME-Version: 1.0 X-WSS-ID: 7CB50AD41QK6483230-01-01 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Tue, 2012-11-20 at 18:36 +0200, Andy Whitcroft wrote: > On Tue, Nov 20, 2012 at 04:14:17PM +0000, Andy Whitcroft wrote: > > On Tue, Nov 20, 2012 at 06:06:10PM +0200, Eilon Greenstein wrote: > > > I'm only testing the nextline if the current line is newly added. If I > > > got it right, when a line is newly added, the next line can be: > > > a. another new line > > > b. existing line (provided for context) > > > c. Does not exist since this is the end of the file (I missed this one > > > originally) > > > > > > It cannot just jump to the next hunk and it cannot be a deleted line, > > > right? > > Oh and in theory at least it could be a - line, though diff never > generates things in that order. > Andy - I could not find any other perl warnings. In the current suggestion, only $line and $prevline are being accessed unconditionally and $rawlines[$linenr] is accessed only after checking it exists - so it seems safe. About the logic - true, if diff will show deleted lines after newly added lines, some new double line segments will be missed. However, it seems like few other things will break if diff will start acting out like that. The suggestion you posted earlier will miss those as well, and starting to check for this weird case (of deleted lines after the added lines) does not seem right. So this patch seems safe, it does not generate false positives and it does catch all the cases we can currently generate with diff - can you please consider applying it to checkpatch? I'm posting it again below for easier reference, please let me know if you would like me to send it in a clean email separately. Thanks, Eilon From 403038375a9c09046631f674d14c221758a0de61 Mon Sep 17 00:00:00 2001 From: Eilon Greenstein Date: Tue, 20 Nov 2012 21:05:30 +0200 Subject: [PATCH] checkpatch: add double empty line check Signed-off-by: Eilon Greenstein --- scripts/checkpatch.pl | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 21a9f5d..c0c610c 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -3579,6 +3579,19 @@ sub process { WARN("EXPORTED_WORLD_WRITABLE", "Exporting world writable files is usually an error. Consider more restrictive permissions.\n" . $herecurr); } + +# check for double empty lines + if ($line =~ /^\+\s*$/) { + my $nextline = ""; + if (defined($rawlines[$linenr])) { + $nextline = $rawlines[$linenr]; + } + if ($nextline =~ /^\s*$/ || + $prevline =~ /^\+?\s*$/ && $nextline !~ /^\+\s*$/) { + CHK("DOUBLE_EMPTY_LINE", + "One empty line should be sufficient. Consider removing this one.\n" . $herecurr); + } + } } # If we have no input at all, then there is nothing to report on