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).
In response to Matthew's request:
This is a possible checkpatch warning for long
function definitions.
Running against the last 10000 git commits, there
are no false positives though perhaps there are
some false negatives.
These are the matches in the last 10000 commits:
1 227 42e0442f8a237d3de9ea3f2dd2be2739e6db7fdb:1157: WARNING:
'ir_lirc_ioctl' function definition is 206 lines, perhaps refactor
2 552 148abd3b5b146021a637d36ac5c0ee91cd4ad520:790: WARNING:
'tda18250_set_params' function definition is 206 lines, perhaps refactor
3 1352 123e25c4a5658be27f08ed0fb85ade34683e5dc7:366: WARNING:
'cudbg_fill_meminfo' function definition is 252 lines, perhaps refactor
4 2688 62d591a8e00cc349e6a9efb87efac9548f178624:462: WARNING:
'program_watermarks' function definition is 232 lines, perhaps refactor
5 6171 d2ddc776a4581d900fc3bdc7803b403daae64d88:3530: WARNING:
'afs_select_fileserver' function definition is 283 lines, perhaps refactor
6 9135 2f4b411a3d6766e6362ffbf00e0495a2dfe92507:962: WARNING:
'i40e_parse_cls_flower' function definition is 242 lines, perhaps refactor
7 9450 fd708b81d972a0714b02a60eb4792fdbf15868c4:1094: WARNING:
'btrfs_ref_tree_mod' function definition is 201 lines, perhaps refactor
Running against files in mm lib and kernel, there are
52 functions that exceed 200 lines.
$ git ls-files -- mm kernel lib | \
xargs ./scripts/checkpatch.pl -f --types=long_function --terse --quiet
--no-summary | \
cat -n
1 kernel/audit.c:1447: WARNING: 'audit_receive_msg' function definition
is 308 lines, perhaps refactor
2 kernel/auditsc.c:713: WARNING: 'audit_filter_rules' function definition
is 273 lines, perhaps refactor
3 kernel/bpf/core.c:1285: WARNING: '___bpf_prog_run' function definition
is 508 lines, perhaps refactor
4 kernel/bpf/verifier.c:2240: WARNING: 'adjust_scalar_min_max_vals'
function definition is 218 lines, perhaps refactor
5 kernel/bpf/verifier.c:4064: WARNING: 'do_check' function definition is
288 lines, perhaps refactor
6 kernel/debug/debug_core.c:682: WARNING: 'kgdb_cpu_enter' function
definition is 214 lines, perhaps refactor
7 kernel/debug/kdb/kdb_io.c:422: WARNING: 'kdb_read' function definition
is 217 lines, perhaps refactor
8 kernel/debug/kdb/kdb_io.c:850: WARNING: 'vkdb_printf' function
definition is 297 lines, perhaps refactor
9 kernel/fork.c:2033: WARNING: 'copy_process' function definition is 454
lines, perhaps refactor
10 kernel/futex.c:705: WARNING: 'get_futex_key' function definition is 204
lines, perhaps refactor
11 kernel/futex.c:2135: WARNING: 'futex_requeue' function definition is
283 lines, perhaps refactor
12 kernel/irq/manage.c:1488: WARNING: '__setup_irq' function definition is
354 lines, perhaps refactor
13 kernel/locking/locktorture.c:1050: WARNING: 'lock_torture_init'
function definition is 201 lines, perhaps refactor
14 kernel/locking/qspinlock.c:505: WARNING: 'queued_spin_lock_slowpath'
function definition is 210 lines, perhaps refactor
15 kernel/locking/rtmutex.c:796: WARNING: 'rt_mutex_adjust_prio_chain'
function definition is 348 lines, perhaps refactor
16 kernel/power/swap.c:873: WARNING: 'save_image_lzo' function definition
is 205 lines, perhaps refactor
17 kernel/power/swap.c:1469: WARNING: 'load_image_lzo' function definition
is 312 lines, perhaps refactor
18 kernel/ptrace.c:1104: WARNING: 'ptrace_request' function definition is
221 lines, perhaps refactor
19 kernel/sched/core.c:4254: WARNING: '_setscheduler' function definition
is 238 lines, perhaps refactor
20 kernel/sched/fair.c:8722: WARNING: 'load_balance' function definition
is 261 lines, perhaps refactor
21 kernel/trace/trace_kprobe.c:839: WARNING: 'create_trace_kprobe'
function definition is 207 lines, perhaps refactor
22 kernel/trace/trace_uprobe.c:564: WARNING: 'create_trace_uprobe'
function definition is 202 lines, perhaps refactor
23 lib/asn1_decoder.c:518: WARNING: 'asn1_ber_decoder' function definition
is 347 lines, perhaps refactor
24 lib/assoc_array.c:790: WARNING: 'assoc_array_insert_into_terminal_node'
function definition is 312 lines, perhaps refactor
25 lib/assoc_array.c:1721: WARNING: 'assoc_array_gc' function definition
is 266 lines, perhaps refactor
26 lib/decompress_bunzip2.c:513: WARNING: 'get_next_block' function
definition is 357 lines, perhaps refactor
27 lib/lz4/lz4_compress.c:456: WARNING: 'LZ4_compress_generic' function
definition is 280 lines, perhaps refactor
28 lib/lz4/lz4_decompress.c:335: WARNING: 'LZ4_decompress_generic'
function definition is 283 lines, perhaps refactor
29 lib/lz4/lz4hc_compress.c:579: WARNING: 'LZ4HC_compress_generic'
function definition is 241 lines, perhaps refactor
30 lib/lzo/lzo1x_decompress_safe.c:261: WARNING: 'lzo1x_decompress_safe'
function definition is 223 lines, perhaps refactor
31 lib/mpi/mpi-pow.c:329: WARNING: 'mpi_powm' function definition is 291
lines, perhaps refactor
32 lib/raid6/recov_avx512.c:230: WARNING: 'raid6_2data_recov_avx512'
function definition is 201 lines, perhaps refactor
33 lib/vsprintf.c:3109: WARNING: 'vsscanf' function definition is 275
lines, perhaps refactor
34 lib/zlib_inflate/inffast.c:347: WARNING: 'inflate_fast' function
definition is 258 lines, perhaps refactor
35 lib/zlib_inflate/inflate.c:740: WARNING: 'zlib_inflate' function
definition is 422 lines, perhaps refactor
36 lib/zlib_inflate/inftrees.c:315: WARNING: 'zlib_inflate_table' function
definition is 292 lines, perhaps refactor
37 lib/zstd/compress.c:830: WARNING: 'ZSTD_compressSequences_internal'
function definition is 243 lines, perhaps refactor
38 lib/zstd/zstd_opt.h:697: WARNING: 'ZSTD_compressBlock_opt_generic'
function definition is 289 lines, perhaps refactor
39 lib/zstd/zstd_opt.h:1012: WARNING:
'ZSTD_compressBlock_opt_extDict_generic' function definition is 311 lines,
perhaps refactor
40 mm/compaction.c:958: WARNING: 'isolate_migratepages_block' function
definition is 268 lines, perhaps refactor
41 mm/filemap.c:2303: WARNING: 'generic_file_buffered_read' function
definition is 251 lines, perhaps refactor
42 mm/khugepaged.c:1560: WARNING: 'collapse_shmem' function definition is
262 lines, perhaps refactor
43 mm/ksm.c:1755: WARNING: 'stable_tree_search' function definition is 236
lines, perhaps refactor
44 mm/memory.c:3080: WARNING: 'do_swap_page' function definition is 210
lines, perhaps refactor
45 mm/mmap.c:968: WARNING: '__vma_adjust' function definition is 287
lines, perhaps refactor
46 mm/nommu.c:1424: WARNING: 'do_mmap' function definition is 223 lines,
perhaps refactor
47 mm/page-writeback.c:1829: WARNING: 'balance_dirty_pages' function
definition is 268 lines, perhaps refactor
48 mm/rmap.c:1609: WARNING: 'try_to_unmap_one' function definition is 275
lines, perhaps refactor
49 mm/shmem.c:1911: WARNING: 'shmem_getpage_gfp' function definition is
315 lines, perhaps refactor
50 mm/swapfile.c:2253: WARNING: 'try_to_unuse' function definition is 222
lines, perhaps refactor
51 mm/swapfile.c:3325: WARNING: 'SYSCALL_DEFINE2' function definition is
230 lines, perhaps refactor
52 mm/vmscan.c:1297: WARNING: 'shrink_page_list' function definition is
411 lines, perhaps refactor
Anyone think this function line length test is useful?
Should it be longer? shorter? not exist at all?
---
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;
