Home » Mailing list archives » linux-kernel » 2011 » January » 4

Re: [patch] checkpatch: putting the && or || on the wrong line

Previous thread: linux-next: Tree for January 4 by Stephen Rothwell on Monday, January 3, 2011 - 10:47 pm. (3 messages)

Next thread: [PATCH 1/1] hidraw: Added Compatibility ioctl() for 32-bit applications. by Alan Ott on Monday, January 3, 2011 - 10:37 pm. (3 messages)
[view original message]
From: Dan Carpenter
Subject: [patch] checkpatch: putting the && or || on the wrong line
Date: Monday, January 3, 2011 - 10:59 pm

This patch makes checkpatch.pl complain if you break up conditions in
the wrong way.

Wrong:
	if ((really_long_condition)
		&& (second_condition)) { ...
Right:
	if ((really_long_condition) &&
		(second_condition)) { ...

If you do it in the wrong way the message is:  "put the && or || at the
end of the previous line"

Signed-off-by: Dan Carpenter <error27@gmail.com>

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index e3c7fc0..0a813db 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1509,6 +1509,11 @@ sub process {
 			WARN("please, no space before tabs\n" . $herevet);
 		}
 
+# check for && or || at the start of a line
+		if ($rawline =~ /^\+\W+(&&|\|\|)/) {
+			WARN("put the && or || at the end of the previous line\n" . $herecurr);
+		}
+
 # check for spaces at the beginning of a line.
 # Exceptions:
 #  1) within comments
--


[ message continues ]
[view original message]
From: Joe Perches
Subject: Re: [patch] checkpatch: putting the && or || on the wrong line
Date: Monday, January 3, 2011 - 11:58 pm

I've submitted something like this a couple of times and gotten
various objections but I think it's sensible.

https://lkml.org/lkml/2009/12/5/65
https://lkml.org/lkml/2010/7/11/92


--


[ message continues ]
[view original message]
From: Dan Carpenter
Subject: Re: [patch] checkpatch: putting the && or || on the wrong line
Date: Tuesday, January 4, 2011 - 2:24 am

If everyone except you thinks it's a waste of time, then stop asking me
to redo my patches.  :/  I don't care either way so long as CodingStyle
and checkpatch reflect the rules.

regards,
dan carpenter

--


[ message continues ]
[view original message]
From: J. Bruce Fields
Subject: Re: [patch] checkpatch: putting the && or || on the wrong line
Date: Tuesday, January 4, 2011 - 9:38 am

As far as I can tell, the convention in mathematical typesetting is to
put operators on the left, not the right.  When the conditions are short
of there are more lines, that allows you to left-align on the repeated
operator.

--


[ message continues ]
[view original message]
From: Samuel Thibault
Subject: Re: [patch] checkpatch: putting the && or || on the wrong line
Date: Tuesday, January 4, 2011 - 9:44 am

I personnally find the left approach more readable.

Samuel
--


[ message continues ]
[view original message]
From: Joe Perches
Subject: Re: [patch] checkpatch: putting the && or || on the wrong line
Date: Tuesday, January 4, 2011 - 10:07 am

As do I, but perhaps coding style in a project like this
shouldn't be personal but collective.

The trailing style outnumbers the leading style ~ 5:1.

$ grep -rP --include=*.[ch] "(\|\||&&)[ \t]*$" * | wc -l
39890
$ grep -rP --include=*.[ch] "^[ \t]*(\|\||&&)" * | wc -l
8244

If you take out drivers/staging, trailing is used ~ 6:1.

I think that high enough to be declared the preferred style.

--


[ message continues ]
Previous thread: linux-next: Tree for January 4 by Stephen Rothwell on Monday, January 3, 2011 - 10:47 pm. (3 messages)

Next thread: [PATCH 1/1] hidraw: Added Compatibility ioctl() for 32-bit applications. by Alan Ott on Monday, January 3, 2011 - 10:37 pm. (3 messages)