Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1543628 > unrolled thread

[PATCH] staging : lustre : Remove braces from single-line body.

Back to article view | Back to linux.kernel

Contents

  [PATCH] staging : lustre : Remove braces from single-line body. Tabrez khan <khan.tabrez21@gmail.com> - 2016-12-16 19:00 +0100
    Re: [PATCH] staging : lustre : Remove braces from single-line body. Joe Perches <joe@perches.com> - 2016-12-16 19:50 +0100
      Re: [PATCH] staging : lustre : Remove braces from single-line body. Dan Carpenter <dan.carpenter@oracle.com> - 2016-12-16 20:00 +0100
        Re: [PATCH] staging : lustre : Remove braces from single-line body. Joe Perches <joe@perches.com> - 2016-12-16 20:20 +0100

#1543628 — [PATCH] staging : lustre : Remove braces from single-line body.

Remove unnecessary braces {} from single line if statement.
This warning is found using checkpatch.pl.

Signed-off-by: Tabrez khan <khan.tabrez21@gmail.com>
---
 drivers/staging/lustre/lustre/ptlrpc/import.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/lustre/lustre/ptlrpc/import.c b/drivers/staging/lustre/lustre/ptlrpc/import.c
index a23d0a0..477d832 100644
--- 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");
-	}
+
 
 	if (lustre_msg_get_last_committed(request->rq_repmsg) > 0 &&
 	    lustre_msg_get_last_committed(request->rq_repmsg) <
-- 
2.7.4

[toc] | [next] | [standalone]

#1543688

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.

And that's an awful lot of !s.

Generically, it might make simpler reading code to
cast to bool instead of using !.

Most code uses !! to make sure whatever value is
either 0 or 1 without changing the logic/polarity.

[toc] | [prev] | [next] | [standalone]

#1543692

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.

regards,
dan carpenter

[toc] | [prev] | [next] | [standalone]

#1543702

[Multipart message — attachments visible in raw view] — view raw

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web