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",
>
