linux-kernel - Re: [RFC patch] checkpatch: Add a test for long function definitions (>200 lines)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CA+55aFxsLpAtPdQn9gq4D+6abLv9w0xkYOBeScpG4RSnKpL1YQ@mail.gmail.com>
Date:   Sun, 17 Dec 2017 13:46:45 -0800
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Joe Perches <joe@...ches.com>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Dan Carpenter <error27@...il.com>,
        Jonathan Corbet <corbet@....net>,
        Matthew Wilcox <willy@...radead.org>
Subject: Re: [RFC patch] checkpatch: Add a test for long function definitions
 (>200 lines)

On Sat, Dec 16, 2017 at 5:26 PM, Joe Perches <joe@...ches.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.

                   LInus