Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757677AbdLQWdH (ORCPT ); Sun, 17 Dec 2017 17:33:07 -0500 Received: from mail-wm0-f49.google.com ([74.125.82.49]:46323 "EHLO mail-wm0-f49.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757609AbdLQWdF (ORCPT ); Sun, 17 Dec 2017 17:33:05 -0500 X-Google-Smtp-Source: ACJfBov1vi44O6HBCwcD4i5X8ZZ8+hpBdR9n5hD9bSEOXlAm2HixKJwEtcL3K28fuzGjzhm4LlVOeg== Date: Sun, 17 Dec 2017 23:33:01 +0100 From: Luc Van Oostenryck To: Linus Torvalds Cc: Joe Perches , Andrew Morton , Linux Kernel Mailing List , Dan Carpenter , Jonathan Corbet , Matthew Wilcox Subject: Re: [RFC patch] checkpatch: Add a test for long function definitions (>200 lines) Message-ID: <20171217223300.463taz4vsnecxx25@ltop.local> References: <20171208223654.GP5858@dastard> <1512838818.26342.7.camel@perches.com> <20171211214300.GT5858@dastard> <1513030348.3036.5.camel@perches.com> <20171211224301.GA3925@bombadil.infradead.org> <1513474017.31581.22.camel@perches.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: NeoMutt/20171027 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2195 Lines: 52 On Sun, Dec 17, 2017 at 01:46:45PM -0800, Linus Torvalds wrote: > On Sat, Dec 16, 2017 at 5:26 PM, Joe Perches wrote: > >> > >> 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. > > So I'm not sure a line count makes sense. > > Sometimes long functions can be sensible, if they are basically just > one big case-statement or similar. > > Looking at one of your examples: futex_requeue() is indeed a long > function, but that's mainly because it has a lot of comments about > exactly what is going on, and while it only has one (fairly small) > case statement, the rest of it is very similar (ie "in this case, do > XYZ"). > > Another case I looked at - try_to_unmap_one() - had very similar > behavior. It's long, but it's not long for the wrong reasons. > > And yes, "copy_process()" is disgusting, and probably _could_ be split > up a bit, but at the same time the bulk of the lines there really is > just the "initialize all the parts of the "struct task_struct". > > And other times, I suspect even a 50-line function is way too dense, > just because it's doing crazy things. > > So I have a really hard time with some arbitrary line limit. At eh > very least, I think it should ignore comments and whitespace lines. > > And yes, some real "complexity analysis" might give a much more sane > limit, but I don't even know what that would be or how it would work. > It would be very easy to let sparse calculate the cyclomatic complexity of each function (and then either printing it or warn if too high), but: - warning would also need a hard limit - cyclomatic complexity of a function with a big (but simple) switch will also be high. I far from sure that the cyclomatic complexity is very useful but maybe some variation of it (like counting a switch as a single edge) could have some value here. -- Luc Van Oostenryck