Signed-off-by: Richard Hartmann <[email protected]>
---
scripts/checkpatch.pl | 32 ++++++++++++++++----------------
1 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index d5405aa..96ca9f0 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1355,7 +1355,7 @@ sub process {
my $ptr = substr($blank, 0, length($utf8_prefix)) . "^";
my $hereptr = "$hereline$ptr\n";
- ERROR("Invalid UTF-8, patch and commit message should be encoded in UTF-8\n" . $hereptr);
+ ERROR("Invalid UTF-8, patch and commit message must be encoded in UTF-8\n" . $hereptr);
}
# ignore non-hunk lines and lines being removed
@@ -1408,7 +1408,7 @@ sub process {
if ($rawline =~ /^\+\s* \t\s*\S/ ||
$rawline =~ /^\+\s* \s*/) {
my $herevet = "$here\n" . cat_vet($rawline) . "\n";
- ERROR("code indent should use tabs where possible\n" . $herevet);
+ ERROR("code indent must use tabs where possible\n" . $herevet);
}
# check we are in a valid C source file if not then ignore this hunk
@@ -1515,7 +1515,7 @@ sub process {
}
}
if ($err ne '') {
- ERROR("switch and case should be at the same indent\n$hereline$err");
+ ERROR("switch and case must be at the same indent\n$hereline$err");
}
}
@@ -1543,7 +1543,7 @@ sub process {
#print "pre<$pre_ctx>\nline<$line>\nctx<$ctx>\nnext<$lines[$ctx_ln - 1]>\n";
if ($ctx !~ /{\s*/ && defined($lines[$ctx_ln -1]) && $lines[$ctx_ln - 1] =~ /^\+\s*{/) {
- ERROR("that open brace { should be on the previous line\n" .
+ ERROR("that open brace { must be on the previous line\n" .
"$here\n$ctx\n$lines[$ctx_ln - 1]\n");
}
if ($level == 0 && $pre_ctx !~ /}\s*while\s*\($/ &&
@@ -1682,7 +1682,7 @@ sub process {
# check for initialisation to aggregates open brace on the next line
if ($line =~ /^.\s*{/ &&
$prevline =~ /(?:^|[^=])=\s*$/) {
- ERROR("that open brace { should be on the previous line\n" . $hereprev);
+ ERROR("that open brace { must be on the previous line\n" . $hereprev);
}
#
@@ -1777,7 +1777,7 @@ sub process {
#print "from<$from> to<$to>\n";
if ($from ne $to) {
- ERROR("\"(foo$from)\" should be \"(foo$to)\"\n" . $herecurr);
+ ERROR("\"(foo$from)\" must be \"(foo$to)\"\n" . $herecurr);
}
} elsif ($line =~ m{\b$NonptrType(\s*(?:$Modifier\b\s*|\*\s*)+)($Ident)}) {
my ($from, $to, $ident) = ($1, $1, $2);
@@ -1794,7 +1794,7 @@ sub process {
#print "from<$from> to<$to> ident<$ident>\n";
if ($from ne $to && $ident !~ /^$Modifier$/) {
- ERROR("\"foo${from}bar\" should be \"foo${to}bar\"\n" . $herecurr);
+ ERROR("\"foo${from}bar\" must be \"foo${to}bar\"\n" . $herecurr);
}
}
@@ -2188,7 +2188,7 @@ sub process {
$stat_real = "[...]\n$stat_real";
}
- ERROR("trailing statements should be on next line\n" . $herecurr . $stat_real);
+ ERROR("trailing statements must be on next line\n" . $herecurr . $stat_real);
}
}
@@ -2212,12 +2212,12 @@ sub process {
my $s = $1;
$s =~ s/$;//g; # Remove any comments
if ($s !~ /^\s*(?:\sif|(?:{|)\s*\\?\s*$)/) {
- ERROR("trailing statements should be on next line\n" . $herecurr);
+ ERROR("trailing statements must be on next line\n" . $herecurr);
}
}
# if should not continue a brace
if ($line =~ /}\s*if\b/) {
- ERROR("trailing statements should be on next line\n" .
+ ERROR("trailing statements must be on next line\n" .
$herecurr);
}
# case and default should not have general statements after them
@@ -2227,14 +2227,14 @@ sub process {
\s*return\s+
)/xg)
{
- ERROR("trailing statements should be on next line\n" . $herecurr);
+ ERROR("trailing statements must be on next line\n" . $herecurr);
}
# Check for }<nl>else {, these must be at the same
# indent level to be relevant to each other.
if ($prevline=~/}\s*$/ and $line=~/^.\s*else\s*/ and
$previndent == $indent) {
- ERROR("else should follow close brace '}'\n" . $hereprev);
+ ERROR("else must follow close brace '}'\n" . $hereprev);
}
if ($prevline=~/}\s*$/ and $line=~/^.\s*while\s*/ and
@@ -2247,7 +2247,7 @@ sub process {
$s =~ s/\n.*//g;
if ($s =~ /^\s*;/) {
- ERROR("while should follow close brace '}'\n" . $hereprev);
+ ERROR("while must follow close brace '}'\n" . $hereprev);
}
}
@@ -2356,7 +2356,7 @@ sub process {
if ($rest !~ /while\s*\(/ &&
$dstat !~ /$exceptions/)
{
- ERROR("Macros with multiple statements should be enclosed in a do - while loop\n" . "$here\n$ctx\n");
+ ERROR("Macros with multiple statements must be enclosed in a do - while loop\n" . "$here\n$ctx\n");
}
} elsif ($ctx !~ /;/) {
@@ -2366,7 +2366,7 @@ sub process {
$dstat !~ /^\.$Ident\s*=/ &&
$dstat =~ /$Operators/)
{
- ERROR("Macros with complex values should be enclosed in parenthesis\n" . "$here\n$ctx\n");
+ ERROR("Macros with complex values must be enclosed in parenthesis\n" . "$here\n$ctx\n");
}
}
}
@@ -2564,7 +2564,7 @@ sub process {
# storage class and type.
if ($line =~ /\b$Type\s+$Inline\b/ ||
$line =~ /\b$Inline\s+$Storage\b/) {
- ERROR("inline keyword should sit between storage class and type\n" . $herecurr);
+ ERROR("inline keyword must sit between storage class and type\n" . $herecurr);
}
# Check for __inline__ and __inline, prefer inline
--
1.6.6.1
On Wed, 10 Feb 2010 01:41:03 +0100 Richard Hartmann wrote:
>
> Signed-off-by: Richard Hartmann <[email protected]>
> ---
Well, I will gladly disagree. checkpatch is an advisory tool.
It has no such final authority to enforce /must/.
/must/ would OK on syntax errors that must be fixed before they will compile.
> scripts/checkpatch.pl | 32 ++++++++++++++++----------------
> 1 files changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index d5405aa..96ca9f0 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -1355,7 +1355,7 @@ sub process {
> my $ptr = substr($blank, 0, length($utf8_prefix)) . "^";
> my $hereptr = "$hereline$ptr\n";
>
> - ERROR("Invalid UTF-8, patch and commit message should be encoded in UTF-8\n" . $hereptr);
> + ERROR("Invalid UTF-8, patch and commit message must be encoded in UTF-8\n" . $hereptr);
> }
>
> # ignore non-hunk lines and lines being removed
> @@ -1408,7 +1408,7 @@ sub process {
> if ($rawline =~ /^\+\s* \t\s*\S/ ||
> $rawline =~ /^\+\s* \s*/) {
> my $herevet = "$here\n" . cat_vet($rawline) . "\n";
> - ERROR("code indent should use tabs where possible\n" . $herevet);
> + ERROR("code indent must use tabs where possible\n" . $herevet);
> }
>
> # check we are in a valid C source file if not then ignore this hunk
> @@ -1515,7 +1515,7 @@ sub process {
> }
> }
> if ($err ne '') {
> - ERROR("switch and case should be at the same indent\n$hereline$err");
> + ERROR("switch and case must be at the same indent\n$hereline$err");
> }
> }
>
> @@ -1543,7 +1543,7 @@ sub process {
> #print "pre<$pre_ctx>\nline<$line>\nctx<$ctx>\nnext<$lines[$ctx_ln - 1]>\n";
>
> if ($ctx !~ /{\s*/ && defined($lines[$ctx_ln -1]) && $lines[$ctx_ln - 1] =~ /^\+\s*{/) {
> - ERROR("that open brace { should be on the previous line\n" .
> + ERROR("that open brace { must be on the previous line\n" .
> "$here\n$ctx\n$lines[$ctx_ln - 1]\n");
> }
> if ($level == 0 && $pre_ctx !~ /}\s*while\s*\($/ &&
> @@ -1682,7 +1682,7 @@ sub process {
> # check for initialisation to aggregates open brace on the next line
> if ($line =~ /^.\s*{/ &&
> $prevline =~ /(?:^|[^=])=\s*$/) {
> - ERROR("that open brace { should be on the previous line\n" . $hereprev);
> + ERROR("that open brace { must be on the previous line\n" . $hereprev);
> }
>
> #
> @@ -1777,7 +1777,7 @@ sub process {
>
> #print "from<$from> to<$to>\n";
> if ($from ne $to) {
> - ERROR("\"(foo$from)\" should be \"(foo$to)\"\n" . $herecurr);
> + ERROR("\"(foo$from)\" must be \"(foo$to)\"\n" . $herecurr);
> }
> } elsif ($line =~ m{\b$NonptrType(\s*(?:$Modifier\b\s*|\*\s*)+)($Ident)}) {
> my ($from, $to, $ident) = ($1, $1, $2);
> @@ -1794,7 +1794,7 @@ sub process {
>
> #print "from<$from> to<$to> ident<$ident>\n";
> if ($from ne $to && $ident !~ /^$Modifier$/) {
> - ERROR("\"foo${from}bar\" should be \"foo${to}bar\"\n" . $herecurr);
> + ERROR("\"foo${from}bar\" must be \"foo${to}bar\"\n" . $herecurr);
> }
> }
>
> @@ -2188,7 +2188,7 @@ sub process {
> $stat_real = "[...]\n$stat_real";
> }
>
> - ERROR("trailing statements should be on next line\n" . $herecurr . $stat_real);
> + ERROR("trailing statements must be on next line\n" . $herecurr . $stat_real);
> }
> }
>
> @@ -2212,12 +2212,12 @@ sub process {
> my $s = $1;
> $s =~ s/$;//g; # Remove any comments
> if ($s !~ /^\s*(?:\sif|(?:{|)\s*\\?\s*$)/) {
> - ERROR("trailing statements should be on next line\n" . $herecurr);
> + ERROR("trailing statements must be on next line\n" . $herecurr);
> }
> }
> # if should not continue a brace
> if ($line =~ /}\s*if\b/) {
> - ERROR("trailing statements should be on next line\n" .
> + ERROR("trailing statements must be on next line\n" .
> $herecurr);
> }
> # case and default should not have general statements after them
> @@ -2227,14 +2227,14 @@ sub process {
> \s*return\s+
> )/xg)
> {
> - ERROR("trailing statements should be on next line\n" . $herecurr);
> + ERROR("trailing statements must be on next line\n" . $herecurr);
> }
>
> # Check for }<nl>else {, these must be at the same
> # indent level to be relevant to each other.
> if ($prevline=~/}\s*$/ and $line=~/^.\s*else\s*/ and
> $previndent == $indent) {
> - ERROR("else should follow close brace '}'\n" . $hereprev);
> + ERROR("else must follow close brace '}'\n" . $hereprev);
> }
>
> if ($prevline=~/}\s*$/ and $line=~/^.\s*while\s*/ and
> @@ -2247,7 +2247,7 @@ sub process {
> $s =~ s/\n.*//g;
>
> if ($s =~ /^\s*;/) {
> - ERROR("while should follow close brace '}'\n" . $hereprev);
> + ERROR("while must follow close brace '}'\n" . $hereprev);
> }
> }
>
> @@ -2356,7 +2356,7 @@ sub process {
> if ($rest !~ /while\s*\(/ &&
> $dstat !~ /$exceptions/)
> {
> - ERROR("Macros with multiple statements should be enclosed in a do - while loop\n" . "$here\n$ctx\n");
> + ERROR("Macros with multiple statements must be enclosed in a do - while loop\n" . "$here\n$ctx\n");
> }
>
> } elsif ($ctx !~ /;/) {
> @@ -2366,7 +2366,7 @@ sub process {
> $dstat !~ /^\.$Ident\s*=/ &&
> $dstat =~ /$Operators/)
> {
> - ERROR("Macros with complex values should be enclosed in parenthesis\n" . "$here\n$ctx\n");
> + ERROR("Macros with complex values must be enclosed in parenthesis\n" . "$here\n$ctx\n");
> }
> }
> }
> @@ -2564,7 +2564,7 @@ sub process {
> # storage class and type.
> if ($line =~ /\b$Type\s+$Inline\b/ ||
> $line =~ /\b$Inline\s+$Storage\b/) {
> - ERROR("inline keyword should sit between storage class and type\n" . $herecurr);
> + ERROR("inline keyword must sit between storage class and type\n" . $herecurr);
> }
>
> # Check for __inline__ and __inline, prefer inline
> --
---
~Randy
On Wed, Feb 10, 2010 at 17:49, Randy Dunlap <[email protected]> wrote:
> Well, I will gladly disagree. Â checkpatch is an advisory tool.
> It has no such final authority to enforce /must/.
>
> /must/ would OK on syntax errors that must be fixed before they will compile.
fwiw, Joe Perches raised the same point in private conversation so there
are two votes against this patch.
I submitted it knowing that it might be controversial, but I went with
the rest of the wording in checkpatch. Other errors have similar
wording, for example prohibited, but that does not mean I am hell-bent
on arguing this point.
Personally, I feel that warnings are suggestions and errors are hard
limits, but it is, of course, OK to disagree with this stance. This is
part of the reason why I submitted it as three separate patches instead
of a single one.
This decision is not mine to make in any case. Dropping it is totally
fine by me.
Thanks for your feedback,
Richard
PS: As I am new to the whole concept of touching the large scary kernel
let me use this opportunity to ask if I should expect answers on the
other patch emails or if they are just merged zsh-style: Silently and
you will notice what went through when you pull a few days later.
On Wed, 10 Feb 2010 18:59:11 +0100 Richard Hartmann wrote:
> PS: As I am new to the whole concept of touching the large scary kernel
> let me use this opportunity to ask if I should expect answers on the
> other patch emails or if they are just merged zsh-style: Silently and
> you will notice what went through when you pull a few days later.
OK, since nobody else has tried to answer this, I'll give it a shot.
It happens many different ways in linux-kernel land, mostly depending on
who is doing the merging, and it's really up to the subsystem or driver
maintainer.
If Andrew Morton merges a patch into his mmotm patchset, you'll get an email
confirmation of it, but this just means that it's in the mmotm patches for
testing, not necessarily for merging into the mainline kernel tree.
If you send a networking patch and David S. Miller merges it, you will get
both an acknowledgement of that and you can see the patch's status at
http://patchwork.ozlabs.org/project/netdev/list/
Same for multimedia driver patches, but view their status at
http://patchwork.kernel.org/project/linux-media/list/
I don't recall if Andy Whitcroft acks checkpatch patches or not, but
you should note that checkpatch is not his highest priority job
(but I'm not trying to speak for him).
If you send patches for the staging or USB or stable pr serial/tty trees,
GregKH will usually ack them when they are merged.
If you send patches for ATA drivers, Jeff Garzik will usually ack them when
they are merged.
If you send patches for the SCSI drivers/subsystem, good luck, the SCSI
mailing list is the best place to find out their status.
If you send patches for parts of Documentation/ that are obviously not
cared for by someone else, I'll try to get to them, but that's not my
highest priority either. I will ack them if/when I merge them.
I guess if it's not clear to you or anyone else, most Linux maintainers
also have other jobs that they usually need to do...
HTH. If you have specific questions, ask away, or maybe Andy can tell us
about checkpatch patches.
---
~Randy
On Thu, Feb 11, 2010 at 06:22, Randy Dunlap <[email protected]> wrote:
> OK, since nobody else has tried to answer this, I'll give it a shot.
> [...]
> HTH.
Thanks a lot. You definitely did help.
I understand that checkpatch is not the highest priority and I am OK
with that. It's just the best way which I can use to give back.
Thanks,
Richard
Richard Hartmann <[email protected]> writes:
> Personally, I feel that warnings are suggestions and errors are hard
> limits,
This is technically impossible - humans are much smarter than the simple
checkpatch script and still every one of us was proven wrong, many times.
These hard limits are not that necessary, though - a fair dose of
flexibility and common sense is an advantage.
--
Krzysztof Halasa