Date: Sun, 17 Dec 2017 23:33:01 +0100
From: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Joe Perches <joe@perches.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Dan Carpenter <error27@gmail.com>, Jonathan Corbet <corbet@lwn.net>,
        Matthew Wilcox <willy@infradead.org>
Subject: Re: [RFC patch] checkpatch: Add a test for long function definitions
 (>200 lines)
Message-ID: <20171217223300.463taz4vsnecxx25@ltop.local>
References: <fd7130d7-9066-524e-1053-a61eeb27cb36@lge.com>
 <Pine.LNX.4.44L0.1712081228430.1371-100000@iolanthe.rowland.org>
 <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>
 <CA+55aFxsLpAtPdQn9gq4D+6abLv9w0xkYOBeScpG4RSnKpL1YQ@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CA+55aFxsLpAtPdQn9gq4D+6abLv9w0xkYOBeScpG4RSnKpL1YQ@mail.gmail.com>
User-Agent: NeoMutt/20171027
Sender: linux-kernel-owner@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 <joe@perches.com> 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