The size of 8 characters used for both TAB and indentation is
embedded as magic value allover the checkpatch script, and this
makes the script less readable.
Replace the magic value 8 with the perl variable "$tabsize".
From the context of the code it's clear if it is used for
indentation or tabulation, so no need to use two separate
variables.
As side benefit of this change, it makes easy to replace the TAB
size when this script is used by other projects with different
requirements in their coding style (e.g. OpenOCD [1] requires
TAB size of 4 characters [2]).
In these cases the script will be probably modified and adapted,
so there is no need (at least for the moment) to expose this
setting on the script's command line.
[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]>
---
scripts/checkpatch.pl | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 373ad345f732..3c8115600516 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) = @_;
@@ -1211,7 +1212,7 @@ sub expand_tabs {
if ($c eq "\t") {
$res .= ' ';
$n++;
- for (; ($n % 8) != 0; $n++) {
+ for (; ($n % $tabsize) != 0; $n++) {
$res .= ' ';
}
next;
@@ -2224,7 +2225,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 +3154,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 +3173,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 +3195,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 +3217,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 +3689,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
On Wed, 2019-05-08 at 14:27 +0200, Antonio Borneo wrote:
> The size of 8 characters used for both TAB and indentation is
> embedded as magic value allover the checkpatch script, and this
> makes the script less readable.
>
> Replace the magic value 8 with the perl variable "$tabsize".
> From the context of the code it's clear if it is used for
> indentation or tabulation, so no need to use two separate
> variables.
>
> As side benefit of this change, it makes easy to replace the TAB
> size when this script is used by other projects with different
> requirements in their coding style (e.g. OpenOCD [1] requires
> TAB size of 4 characters [2]).
> In these cases the script will be probably modified and adapted,
> so there is no need (at least for the moment) to expose this
> setting on the script's command line.
Disagree. Probably getter to add a --tabsize=<foo> option now.
On Wed, 2019-05-08 at 17:32 +0200, Antonio Borneo wrote:
> On Wed, May 8, 2019 at 4:52 PM Joe Perches <[email protected]> wrote:
> ...
> > > In these cases the script will be probably modified and adapted,
> > > so there is no need (at least for the moment) to expose this
> > > setting on the script's command line.
> >
> > Disagree. Probably getter to add a --tabsize=<foo> option now.
>
> Ok, will send a V2 including the command line option.
> Exposing TAB size, makes the option name relevant; should I keep
> "--tabsize" or is "--tab-stop" more appropriate?
--tabsize is probably more appropriate as tab stops are not
always a multiple of a single number.
Unless you really want to get funky and support something like
--tab-stops=7,13,17,...
I don't suggest that.
On Wed, May 8, 2019 at 4:52 PM Joe Perches <[email protected]> wrote:
...
> > In these cases the script will be probably modified and adapted,
> > so there is no need (at least for the moment) to expose this
> > setting on the script's command line.
>
> Disagree. Probably getter to add a --tabsize=<foo> option now.
Ok, will send a V2 including the command line option.
Exposing TAB size, makes the option name relevant; should I keep
"--tabsize" or is "--tab-stop" more appropriate?
Antonio
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
v4 -> v5
exclude --tab-size=1
v5 -> v6
rebased
---
scripts/checkpatch.pl | 26 ++++++++++++++++----------
1 file changed, 16 insertions(+), 10 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 4c1be774b0ed..8bac825665ef 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -64,6 +64,7 @@ my $color = "auto";
my $allow_c99_comments = 1; # Can be overridden by --ignore C99_COMMENT_TOLERANCE
# git output parsing needs US English output, so first set backtick child process LANGUAGE
my $git_command ='export LANGUAGE=en_US.UTF-8; git';
+my $tabsize = 8;
sub help {
my ($exitcode) = @_;
@@ -98,6 +99,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
@@ -215,6 +217,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,
@@ -267,6 +270,9 @@ if ($color =~ /^[01]$/) {
die "Invalid color mode: $color\n";
}
+# skip TAB size 1 to avoid additional checks on $tabsize - 1
+die "Invalid TAB size: $tabsize\n" if ($tabsize < 2);
+
sub hash_save_array_words {
my ($hashRef, $arrayRef) = @_;
@@ -1217,7 +1223,7 @@ sub expand_tabs {
if ($c eq "\t") {
$res .= ' ';
$n++;
- for (; ($n % 8) != 0; $n++) {
+ for (; ($n % $tabsize) != 0; $n++) {
$res .= ' ';
}
next;
@@ -2230,7 +2236,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;
@@ -3198,7 +3204,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";
@@ -3217,7 +3223,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/) {}
}
@@ -3239,11 +3245,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;
}
}
}
@@ -3261,8 +3267,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 &&
@@ -3733,11 +3739,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.25.0