All of lore.kernel.org
 help / color / mirror / Atom feed
From: Joe Perches <joe@perches.com>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: Tabrez khan <khan.tabrez21@gmail.com>,
	oleg.drokin@intel.com, jsimmons@infradead.org,
	andreas.dilger@intel.com, devel@driverdev.osuosl.org,
	gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org,
	lustre-devel@lists.lustre.org
Subject: [lustre-devel] [PATCH] staging : lustre : Remove braces from single-line body.
Date: Fri, 16 Dec 2016 11:19:22 -0800	[thread overview]
Message-ID: <1481915962.29291.96.camel@perches.com> (raw)
In-Reply-To: <20161216185355.GH8176@mwanda>

On Fri, 2016-12-16 at 21:53 +0300, Dan Carpenter wrote:
> On Fri, Dec 16, 2016 at 10:43:24AM -0800, Joe Perches wrote:
> > On Fri, 2016-12-16 at 23:29 +0530, Tabrez khan wrote:
> > > Remove unnecessary braces {} from single line if statement.
> > > This warning is found using checkpatch.pl.
> > 
> > []
> > > diff --git a/drivers/staging/lustre/lustre/ptlrpc/import.c b/drivers/staging/lustre/lustre/ptlrpc/import.c
> > 
> > []
> > > @@ -1134,9 +1134,9 @@ static int ptlrpc_connect_interpret(const struct lu_env *env,
> > >  	}
> > >  
> > >  	/* Sanity checks for a reconnected import. */
> > > -	if (!(imp->imp_replayable) != !(msg_flags & MSG_CONNECT_REPLAYABLE)) {
> > > +	if (!(imp->imp_replayable) != !(msg_flags & MSG_CONNECT_REPLAYABLE))
> > >  		CERROR("imp_replayable flag does not match server after reconnect. We should LBUG right here.\n");
> > > -	}
> > > +
> > 
> > There are one too many blank lines now.
> 
> I was expecting checkpatch.pl to catch the extra blank.  It was there in
> the last patch as well.  Apparently it doesn't.

checkpatch is imperfect and always will be.

checkpatch is a stupid little script.

It mostly works on added lines and generally
only looks for style defects on lines that
precede those added lines.

Here the now unnecessary blank line follows the
added blank line.

Oh well.

Anyway, here's an almost completely untested
possible patch to checkpatch.

(also attached because evolution is a terrible
 email client, but I seem to be stuck with it)

---
 scripts/checkpatch.pl | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 1579230ebacc..97078f7629e2 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3063,6 +3063,19 @@ sub process {
 			$last_blank_line = $linenr;
 		}
 
+# Another check for multiple consecutive blank lines where the following
+# line is part of the context and is also blank
+		if ($line =~ /^\+\s*$/ &&
+		    defined $rawlines[$linenr] &&
+		    $rawlines[$linenr] =~ /^ \s*$/) {
+			if (CHK("LINE_SPACING",
+				"Please don't use multiple blank lines\n" . $hereprev) &&
+			    $fix) {
+				fix_delete_line($fixlinenr, $rawline);
+			}
+			$last_blank_line = $linenr;
+		}
+
 # check for missing blank lines after declarations
 		if ($sline =~ /^\+\s+\S/ &&			#Not at char 1
 			# actual declarations
-------------- next part --------------
A non-text attachment was scrubbed...
Name: cp_trailing_context_blank.diff
Type: text/x-patch
Size: 860 bytes
Desc: not available
URL: <http://lists.lustre.org/pipermail/lustre-devel-lustre.org/attachments/20161216/63a30696/attachment.bin>

WARNING: multiple messages have this Message-ID (diff)
From: Joe Perches <joe@perches.com>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: Tabrez khan <khan.tabrez21@gmail.com>,
	oleg.drokin@intel.com, jsimmons@infradead.org,
	andreas.dilger@intel.com, devel@driverdev.osuosl.org,
	gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org,
	lustre-devel@lists.lustre.org
Subject: Re: [PATCH] staging : lustre : Remove braces from single-line body.
Date: Fri, 16 Dec 2016 11:19:22 -0800	[thread overview]
Message-ID: <1481915962.29291.96.camel@perches.com> (raw)
In-Reply-To: <20161216185355.GH8176@mwanda>

[-- Attachment #1: Type: text/plain, Size: 2362 bytes --]

On Fri, 2016-12-16 at 21:53 +0300, Dan Carpenter wrote:
> On Fri, Dec 16, 2016 at 10:43:24AM -0800, Joe Perches wrote:
> > On Fri, 2016-12-16 at 23:29 +0530, Tabrez khan wrote:
> > > Remove unnecessary braces {} from single line if statement.
> > > This warning is found using checkpatch.pl.
> > 
> > []
> > > diff --git a/drivers/staging/lustre/lustre/ptlrpc/import.c b/drivers/staging/lustre/lustre/ptlrpc/import.c
> > 
> > []
> > > @@ -1134,9 +1134,9 @@ static int ptlrpc_connect_interpret(const struct lu_env *env,
> > >  	}
> > >  
> > >  	/* Sanity checks for a reconnected import. */
> > > -	if (!(imp->imp_replayable) != !(msg_flags & MSG_CONNECT_REPLAYABLE)) {
> > > +	if (!(imp->imp_replayable) != !(msg_flags & MSG_CONNECT_REPLAYABLE))
> > >  		CERROR("imp_replayable flag does not match server after reconnect. We should LBUG right here.\n");
> > > -	}
> > > +
> > 
> > There are one too many blank lines now.
> 
> I was expecting checkpatch.pl to catch the extra blank.  It was there in
> the last patch as well.  Apparently it doesn't.

checkpatch is imperfect and always will be.

checkpatch is a stupid little script.

It mostly works on added lines and generally
only looks for style defects on lines that
precede those added lines.

Here the now unnecessary blank line follows the
added blank line.

Oh well.

Anyway, here's an almost completely untested
possible patch to checkpatch.

(also attached because evolution is a terrible
 email client, but I seem to be stuck with it)

---
 scripts/checkpatch.pl | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 1579230ebacc..97078f7629e2 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3063,6 +3063,19 @@ sub process {
 			$last_blank_line = $linenr;
 		}
 
+# Another check for multiple consecutive blank lines where the following
+# line is part of the context and is also blank
+		if ($line =~ /^\+\s*$/ &&
+		    defined $rawlines[$linenr] &&
+		    $rawlines[$linenr] =~ /^ \s*$/) {
+			if (CHK("LINE_SPACING",
+				"Please don't use multiple blank lines\n" . $hereprev) &&
+			    $fix) {
+				fix_delete_line($fixlinenr, $rawline);
+			}
+			$last_blank_line = $linenr;
+		}
+
 # check for missing blank lines after declarations
 		if ($sline =~ /^\+\s+\S/ &&			#Not at char 1
 			# actual declarations

[-- Attachment #2: cp_trailing_context_blank.diff --]
[-- Type: text/x-patch, Size: 860 bytes --]

 scripts/checkpatch.pl | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 1579230ebacc..97078f7629e2 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3063,6 +3063,19 @@ sub process {
 			$last_blank_line = $linenr;
 		}
 
+# Another check for multiple consecutive blank lines where the following
+# line is part of the context and is also blank
+		if ($line =~ /^\+\s*$/ &&
+		    defined $rawlines[$linenr] &&
+		    $rawlines[$linenr] =~ /^ \s*$/) {
+			if (CHK("LINE_SPACING",
+				"Please don't use multiple blank lines\n" . $hereprev) &&
+			    $fix) {
+				fix_delete_line($fixlinenr, $rawline);
+			}
+			$last_blank_line = $linenr;
+		}
+
 # check for missing blank lines after declarations
 		if ($sline =~ /^\+\s+\S/ &&			#Not at char 1
 			# actual declarations

  reply	other threads:[~2016-12-16 19:19 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-16 17:59 [lustre-devel] [PATCH] staging : lustre : Remove braces from single-line body Tabrez khan
2016-12-16 17:59 ` Tabrez khan
2016-12-16 18:43 ` [lustre-devel] " Joe Perches
2016-12-16 18:43   ` Joe Perches
2016-12-16 18:53   ` [lustre-devel] " Dan Carpenter
2016-12-16 18:53     ` Dan Carpenter
2016-12-16 19:19     ` Joe Perches [this message]
2016-12-16 19:19       ` Joe Perches
  -- strict thread matches above, loose matches on Subject: below --
2016-12-29 20:02 [lustre-devel] [PATCH] staging:lustre:remove " Tabrez khan
2016-12-28 14:10 [lustre-devel] [PATCH] staging : lustre : Remove " Tabrez khan
2017-01-03 14:15 ` Greg KH
2016-12-16 14:29 Tabrez khan
2016-12-16 16:05 ` Joe Perches
2016-12-16 17:23   ` Xiong, Jinshan
2016-12-16 17:23     ` Xiong, Jinshan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1481915962.29291.96.camel@perches.com \
    --to=joe@perches.com \
    --cc=andreas.dilger@intel.com \
    --cc=dan.carpenter@oracle.com \
    --cc=devel@driverdev.osuosl.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=jsimmons@infradead.org \
    --cc=khan.tabrez21@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lustre-devel@lists.lustre.org \
    --cc=oleg.drokin@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.