Re: [PATCH v4 72/73] xfs: Convert mru cache to XArray

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 2017-12-11 at 14:43 -0800, Matthew Wilcox wrote:
>  - There's no warning for the first paragraph of section 6:
> 
> 6) Functions
> ------------
> 
> Functions should be short and sweet, and do just one thing.  They should
> fit on one or two screenfuls of text (the ISO/ANSI screen size is 80x24,
> as we all know), and do one thing and do that well.
> 
>    I'm not expecting you to be able to write a perl script that checks
>    the first line, but we have way too many 200-plus line functions in
>    the kernel.  I'd like a warning on anything over 200 lines (a factor
>    of 4 over Linus's stated goal).

Perhaps something like this?

(very very lightly tested, more testing appreciated)
---
 scripts/checkpatch.pl | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 4306b7616cdd..523aead40b87 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -59,6 +59,7 @@ my $conststructsfile = "$D/const_structs.checkpatch";
 my $typedefsfile = "";
 my $color = "auto";
 my $allow_c99_comments = 1;
+my $max_function_length = 200;
 
 sub help {
 	my ($exitcode) = @_;
@@ -2202,6 +2203,7 @@ sub process {
 	my $realcnt = 0;
 	my $here = '';
 	my $context_function;		#undef'd unless there's a known function
+	my $context_function_linenum;
 	my $in_comment = 0;
 	my $comment_edge = 0;
 	my $first_line = 0;
@@ -2341,6 +2343,7 @@ sub process {
 			} else {
 				undef $context_function;
 			}
+			undef $context_function_linenum;
 			next;
 
 # track the line number as we move through the hunk, note that
@@ -3200,11 +3203,18 @@ sub process {
 		if ($sline =~ /^\+\{\s*$/ &&
 		    $prevline =~ /^\+(?:(?:(?:$Storage|$Inline)\s*)*\s*$Type\s*)?($Ident)\(/) {
 			$context_function = $1;
+			$context_function_linenum = $realline;
 		}
 
 # check if this appears to be the end of function declaration
 		if ($sline =~ /^\+\}\s*$/) {
+			if (defined($context_function_linenum) &&
+			    ($realline - $context_function_linenum) > $max_function_length) {
+				WARN("LONG_FUNCTION",
+				     "'$context_function' function definition is " . ($realline - $context_function_linenum) . " lines, perhaps refactor\n" . $herecurr);
+			}
 			undef $context_function;
+			undef $context_function_linenum;
 		}
 
 # check indentation of any line with a bare else
@@ -5983,6 +5993,7 @@ sub process {
 		    defined $stat &&
 		    $stat =~ /^.\s*(?:$Storage\s+)?$Type\s*($Ident)\s*$balanced_parens\s*{/s) {
 			$context_function = $1;
+			$context_function_linenum = $realline;
 
 # check for multiline function definition with misplaced open brace
 			my $ok = 0;

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux