2019-05-09 07:22:52

by Antonio Borneo

[permalink] [raw]
Subject: [PATCH v4] checkpatch: add command-line option for TAB size

Linux kernel coding style requires a size of 8 characters for
both TAB and indentation, and such value is embedded as magic
value allover the checkpatch script.
This makes hard to reuse the script by other projects with
different requirements in their coding style (e.g. OpenOCD [1]
requires TAB size of 4 characters [2]).

Replace the magic value 8 with a variable.

Add a command-line option "--tab-size" to let the user select a
TAB size value other than 8.

[1] http://openocd.org/
[2] http://openocd.org/doc/doxygen/html/stylec.html#styleformat

Signed-off-by: Antonio Borneo <[email protected]>
Signed-off-by: Erik AhlĂ©n <[email protected]>
Signed-off-by: Spencer Oliver <[email protected]>
---
v1 -> v2
add the command line option

v2 -> v3
rewrite commit msg to remove script readability issue

v3 -> v4
check for command line value positive
---
scripts/checkpatch.pl | 25 +++++++++++++++----------
1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 916a3fbd4d47..216f2c8db7aa 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -62,6 +62,7 @@ my $conststructsfile = "$D/const_structs.checkpatch";
my $typedefsfile = "";
my $color = "auto";
my $allow_c99_comments = 1; # Can be overridden by --ignore C99_COMMENT_TOLERANCE
+my $tabsize = 8;

sub help {
my ($exitcode) = @_;
@@ -96,6 +97,7 @@ Options:
--show-types show the specific message type in the output
--max-line-length=n set the maximum line length, if exceeded, warn
--min-conf-desc-length=n set the min description length, if shorter, warn
+ --tab-size=n set the number of spaces for tab (default 8)
--root=PATH PATH to the kernel tree root
--no-summary suppress the per-file summary
--mailback only produce a report in case of warnings/errors
@@ -213,6 +215,7 @@ GetOptions(
'list-types!' => \$list_types,
'max-line-length=i' => \$max_line_length,
'min-conf-desc-length=i' => \$min_conf_desc_length,
+ 'tab-size=i' => \$tabsize,
'root=s' => \$root,
'summary!' => \$summary,
'mailback!' => \$mailback,
@@ -265,6 +268,8 @@ if ($color =~ /^[01]$/) {
die "Invalid color mode: $color\n";
}

+die "Invalid TAB size: $tabsize\n" if ($tabsize <= 0);
+
sub hash_save_array_words {
my ($hashRef, $arrayRef) = @_;

@@ -1211,7 +1216,7 @@ sub expand_tabs {
if ($c eq "\t") {
$res .= ' ';
$n++;
- for (; ($n % 8) != 0; $n++) {
+ for (; ($n % $tabsize) != 0; $n++) {
$res .= ' ';
}
next;
@@ -2224,7 +2229,7 @@ sub string_find_replace {
sub tabify {
my ($leading) = @_;

- my $source_indent = 8;
+ my $source_indent = $tabsize;
my $max_spaces_before_tab = $source_indent - 1;
my $spaces_to_tab = " " x $source_indent;

@@ -3153,7 +3158,7 @@ sub process {
next if ($realfile !~ /\.(h|c|pl|dtsi|dts)$/);

# at the beginning of a line any tabs must come first and anything
-# more than 8 must use tabs.
+# more than $tabsize must use tabs.
if ($rawline =~ /^\+\s* \t\s*\S/ ||
$rawline =~ /^\+\s* \s*/) {
my $herevet = "$here\n" . cat_vet($rawline) . "\n";
@@ -3172,7 +3177,7 @@ sub process {
"please, no space before tabs\n" . $herevet) &&
$fix) {
while ($fixed[$fixlinenr] =~
- s/(^\+.*) {8,8}\t/$1\t\t/) {}
+ s/(^\+.*) {$tabsize,$tabsize}\t/$1\t\t/) {}
while ($fixed[$fixlinenr] =~
s/(^\+.*) +\t/$1\t/) {}
}
@@ -3194,11 +3199,11 @@ sub process {
if ($perl_version_ok &&
$sline =~ /^\+\t+( +)(?:$c90_Keywords\b|\{\s*$|\}\s*(?:else\b|while\b|\s*$)|$Declare\s*$Ident\s*[;=])/) {
my $indent = length($1);
- if ($indent % 8) {
+ if ($indent % $tabsize) {
if (WARN("TABSTOP",
"Statements should start on a tabstop\n" . $herecurr) &&
$fix) {
- $fixed[$fixlinenr] =~ s@(^\+\t+) +@$1 . "\t" x ($indent/8)@e;
+ $fixed[$fixlinenr] =~ s@(^\+\t+) +@$1 . "\t" x ($indent/$tabsize)@e;
}
}
}
@@ -3216,8 +3221,8 @@ sub process {
my $newindent = $2;

my $goodtabindent = $oldindent .
- "\t" x ($pos / 8) .
- " " x ($pos % 8);
+ "\t" x ($pos / $tabsize) .
+ " " x ($pos % $tabsize);
my $goodspaceindent = $oldindent . " " x $pos;

if ($newindent ne $goodtabindent &&
@@ -3688,11 +3693,11 @@ sub process {
#print "line<$line> prevline<$prevline> indent<$indent> sindent<$sindent> check<$check> continuation<$continuation> s<$s> cond_lines<$cond_lines> stat_real<$stat_real> stat<$stat>\n";

if ($check && $s ne '' &&
- (($sindent % 8) != 0 ||
+ (($sindent % $tabsize) != 0 ||
($sindent < $indent) ||
($sindent == $indent &&
($s !~ /^\s*(?:\}|\{|else\b)/)) ||
- ($sindent > $indent + 8))) {
+ ($sindent > $indent + $tabsize))) {
WARN("SUSPECT_CODE_INDENT",
"suspect code indent for conditional statements ($indent, $sindent)\n" . $herecurr . "$stat_real\n");
}
--
2.21.0


2019-05-09 08:06:22

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH v4] checkpatch: add command-line option for TAB size

On Thu, 2019-05-09 at 09:21 +0200, Antonio Borneo wrote:
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> @@ -2224,7 +2229,7 @@ sub string_find_replace {
> sub tabify {
> my ($leading) = @_;
>
> - my $source_indent = 8;
> + my $source_indent = $tabsize;
> my $max_spaces_before_tab = $source_indent - 1;

I didn't test this.

Does this work properly if --tab-size=1 ?
Maybe die if ($tabsize < 2); is necessary?


2019-05-09 09:29:41

by Antonio Borneo

[permalink] [raw]
Subject: Re: [PATCH v4] checkpatch: add command-line option for TAB size

On Thu, May 9, 2019 at 10:03 AM Joe Perches <[email protected]> wrote:
>
> On Thu, 2019-05-09 at 09:21 +0200, Antonio Borneo wrote:
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> []
> > @@ -2224,7 +2229,7 @@ sub string_find_replace {
> > sub tabify {
> > my ($leading) = @_;
> >
> > - my $source_indent = 8;
> > + my $source_indent = $tabsize;
> > my $max_spaces_before_tab = $source_indent - 1;
>
> I didn't test this.
>
> Does this work properly if --tab-size=1 ?
> Maybe die if ($tabsize < 2); is necessary?

I have tested it and it works fine, but now that you point it, I
rechecked the code. There is already this in checkpatch
sub tabify {
...
my $source_indent = $tabsize;
my $max_spaces_before_tab = $source_indent - 1;
...
#Remove spaces before a tab
1 while $leading =~ s@^([\t]*)( {1,$max_spaces_before_tab})\t@$1\t@g;
that works fine.
But we could have in the future some other test in checkpatch that
uses "$tabsize - 1" and does not get the proper validation for
--tab-size=1
Maybe it's safer to put die if ($tabsize < 2) right now, and avoid
future headaches. With a comment to explains this choice.

Antonio