From: Joe Perches <joe@perches.com>
To: Matthew Wilcox <willy@infradead.org>
Cc: Christoph Hellwig <hch@lst.de>,
apw@canonical.com, Linus Torvalds <torvalds@linux-foundation.org>,
Andrew Morton <akpm@linux-foundation.org>,
linux-kernel@vger.kernel.org,
linux-doc <linux-doc@vger.kernel.org>,
Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
Subject: Re: [PATCH] checkpatch: make the line length warnings match the coding style document
Date: Mon, 21 Dec 2020 20:08:20 -0800 [thread overview]
Message-ID: <93a470c7631d2607e7b2a12e9cc5d8e930911989.camel@perches.com> (raw)
In-Reply-To: <4898c0c03d370a23b1b98ddabb72e70ec8d430fa.camel@perches.com>
On Thu, 2020-12-10 at 13:27 -0800, Joe Perches wrote:
> On Thu, 2020-12-10 at 20:09 +0000, Matthew Wilcox wrote:
> > On Thu, Dec 10, 2020 at 12:05:04PM -0800, Joe Perches wrote:
> > > Also, given the ever increasing average identifier length, strict
> > > adherence to 80 columns is sometimes just not possible without silly
> > > visual gymnastics. The kernel now has quite a lot of 30+ character
> > > length function names, constants, and structs.
> >
> > maybe checkpatch should warn for identifiers that are 30+ characters
> > long? address the problem at its source ..
>
> Hard to know when to warn as patches could just add uses of already
> existing names and emitting warnings for those would just be annoying.
>
> Maybe something that tests long identifier additions of
> defines/functions/macros/structs but not their uses and maybe only
> then in patches and not files.
>
> Perhaps:
Anyone care that this should be added or not added to checkpatch?
> ---
> scripts/checkpatch.pl | 32 ++++++++++++++++++++++++++++++++
> 1 file changed, 32 insertions(+)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 7b086d1cd6c2..8579be987fc0 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -54,6 +54,7 @@ my @ignore = ();
> my $help = 0;
> my $configuration_file = ".checkpatch.conf";
> my $max_line_length = 100;
> +my $max_identifier_length = 30;
> my $ignore_perl_version = 0;
> my $minimum_perl_version = 5.10.0;
> my $min_conf_desc_length = 4;
> @@ -103,6 +104,8 @@ Options:
> --max-line-length=n set the maximum line length, (default $max_line_length)
> if exceeded, warn on patches
> requires --strict for use with --file
> + --max-identifier-length=n set the maximum identifier length, (default $max_identifier_length)
> + only used with patches, not output with --file
> --min-conf-desc-length=n set the min description length, if shorter, warn
> --tab-size=n set the number of spaces for tab (default $tabsize)
> --root=PATH PATH to the kernel tree root
> @@ -223,6 +226,7 @@ GetOptions(
> 'show-types!' => \$show_types,
> 'list-types!' => \$list_types,
> 'max-line-length=i' => \$max_line_length,
> + 'max-identifier-length=i' => \$max_identifier_length,
> 'min-conf-desc-length=i' => \$min_conf_desc_length,
> 'tab-size=i' => \$tabsize,
> 'root=s' => \$root,
> @@ -2489,6 +2493,7 @@ sub process {
> my $suppress_statement = 0;
>
>
> my %signatures = ();
> + my %long_identifiers = ();
>
>
> # Pre-scan the patch sanitizing the lines.
> # Pre-scan the patch looking for any __setup documentation.
> @@ -3840,6 +3845,33 @@ sub process {
> # check we are in a valid C source file if not then ignore this hunk
> next if ($realfile !~ /\.(h|c)$/);
>
>
> +# check for long identifiers in defines/macros/functions/structs/types/labels
> + if (!$file) {
> + while ($sline =~ /^\+.*\b(\w{$max_identifier_length,})\b/g) {
> + my $id = $1;
> + next if (exists($long_identifiers{$id}));
> + my $use = "";
> + if ($sline =~ /^\+\s*\#\s*define\s+$id(?!\()/) {
> + $use = "define";
> + } elsif ($sline =~ /^\+\s*\#\s*define\s+$id\(/) {
> + $use = "function-like macro";
> + } elsif ($sline =~ /^\+\s*(?!define)$Declare?$id\s*\(/) {
> + $use = "function";
> + } elsif ($sline =~ /^\+\s*(struct|union|enum)\s+$id\b/) {
> + $use = "$1";
> + } elsif ($sline =~ /^\+\s*$Declare$id\b/) {
> + $use = "declaration";
> + } elsif ($sline =~ /^\+\s*$id\s*:\s*$/) {
> + $use = "label";
> + }
> + if ($use ne "") {
> + $long_identifiers{$id} = $id;
> + WARN("LONG_IDENTIFIER",
> + "$use '$id' is " . length($id) . " characters - avoid using identifiers with $max_identifier_length+ characters\n" . $herecurr);
> + }
> + }
> + }
> +
> # check for unusual line ending [ or (
> if ($line =~ /^\+.*([\[\(])\s*$/) {
> CHK("OPEN_ENDED_LINE",
>
next prev parent reply other threads:[~2020-12-22 4:09 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-10 8:22 [PATCH] checkpatch: make the line length warnings match the coding style document Christoph Hellwig
2020-12-10 20:05 ` Joe Perches
2020-12-10 20:09 ` Matthew Wilcox
2020-12-10 21:27 ` Joe Perches
2020-12-22 4:08 ` Joe Perches [this message]
2020-12-22 13:12 ` Christoph Hellwig
2020-12-22 16:22 ` Joe Perches
2020-12-23 8:29 ` Christoph Hellwig
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=93a470c7631d2607e7b2a12e9cc5d8e930911989.camel@perches.com \
--to=joe@perches.com \
--cc=akpm@linux-foundation.org \
--cc=apw@canonical.com \
--cc=hch@lst.de \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=miguel.ojeda.sandonis@gmail.com \
--cc=torvalds@linux-foundation.org \
--cc=willy@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).