Re: clean up kernel_{read,write} & friends v2
From: Joe Perches <joe@perches.com>
Date: 2020-05-28 19:29:36
Also in:
linux-fsdevel, lkml, netfilter-devel
On Thu, 2020-05-28 at 11:51 -0700, Linus Torvalds wrote:
On Wed, May 27, 2020 at 10:40 PM Christoph Hellwig [off-list ref] wrote:quoted
this series fixes a few issues and cleans up the helpers that read from or write to kernel space buffers, and ensures that we don't change the address limit if we are using the ->read_iter and ->write_iter methods that don't need the changed address limit.Apart from the "please don't mix irrelevant whitespace changes with other changes" comment, this looks fine to me. And a rant related to that change: I'm really inclined to remove the checkpatch check for 80 columns entirely, but it shouldn't have been triggering for old lines even now. Or maybe make it check for something more reasonable, like 100 characters. I find it ironic and annoying how "checkpatch" warns about that silly legacy limit, when checkpatch itself then on the very next few lines has a line that is 124 columns wide
Yeah. perl ain't c. And this discussion has been had many times. Here's one from 2009 https://lkml.org/lkml/2009/12/15/490 Another from 2012 https://lkml.org/lkml/2012/2/5/141 Line lengths checks are normally pretty silly. Hard limits at 80 really don't work well, especially with some of the 25+ character length identifiers used today. I think a line length warning at 132 is generally reasonable but it could depend on complexity and identifier lengths.
And yes, that 124 character line has a good reason for it. But that's kind of the point. There are lots of perfectly fine reasons for longer lines. I'd much rather check for "no deep indentation" or "no unnecessarily complex conditionals" or other issues that are more likely to be _real_ problems.
That deep indentation test already exists at 6 tabs.
Maybe it should be 5 instead. Or maybe even 4, but
that's a pretty easy to need and common use case.
Tab depth use in the kernel is more or less
$ git grep -Poh '^\t+(if|do|while|for|switch)\b' | \
sed -r 's/\w+//g' | \
awk '{print length($0);}' | \
sort | uniq -c | sort -rn
903993 1
339059 2
89334 3
18216 4
3282 5
605 6
148 7
36 8
4 9
1 10
But do we really have 80x25 terminals any more that we'd care about?
trivial btw: VT100s were 80x24 or 132x24, PCs were 80x25