From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757222AbdLQB1C (ORCPT ); Sat, 16 Dec 2017 20:27:02 -0500 Received: from smtprelay0126.hostedemail.com ([216.40.44.126]:53638 "EHLO smtprelay.hostedemail.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1757174AbdLQB1A (ORCPT ); Sat, 16 Dec 2017 20:27:00 -0500 X-Session-Marker: 6A6F6540706572636865732E636F6D X-Spam-Summary: 30,2,0,,d41d8cd98f00b204,joe@perches.com,:::::::::::,RULES_HIT:1:41:69:355:379:541:973:982:988:989:1260:1277:1311:1313:1314:1345:1359:1373:1437:1515:1516:1518:1593:1594:1605:1730:1747:1777:1792:1981:2194:2197:2198:2199:2200:2201:2393:2559:2562:2636:2828:3138:3139:3140:3141:3142:3622:3653:3865:3866:3867:3868:3870:3871:3872:3873:3874:4321:5007:6120:7875:7903:8603:10004:10226:10848:11026:11232:11658:11914:12043:12438:12555:12663:12740:12760:12895:12986:13161:13229:13439:14659:21063:21080:21221:21433:21451:21505:21611:21627:30034:30054:30070:30091,0,RBL:none,CacheIP:none,Bayesian:0.5,0.5,0.5,Netcheck:none,DomainCache:0,MSF:not bulk,SPF:,MSBL:0,DNSBL:none,Custom_rules:0:0:0,LFtime:1,LUA_SUMMARY:none X-HE-Tag: fold97_2c585f43b32d X-Filterd-Recvd-Size: 11314 Message-ID: <1513474017.31581.22.camel@perches.com> Subject: [RFC patch] checkpatch: Add a test for long function definitions (>200 lines) From: Joe Perches To: Andrew Morton Cc: linux-kernel@vger.kernel.org, Dan Carpenter , Linus Torvalds , Jonathan Corbet , Matthew Wilcox Date: Sat, 16 Dec 2017 17:26:57 -0800 In-Reply-To: <20171211224301.GA3925@bombadil.infradead.org> References: <20171208223654.GP5858@dastard> <1512838818.26342.7.camel@perches.com> <20171211214300.GT5858@dastard> <1513030348.3036.5.camel@perches.com> <20171211224301.GA3925@bombadil.infradead.org> Content-Type: text/plain; charset="ISO-8859-1" X-Mailer: Evolution 3.26.1-1 Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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;