* [Qemu-devel] [PATCH] CHECKPATCH: Add warning for single else statement.
2012-09-01 15:57 [Qemu-devel] [PATCH] CHECKPATCH Don Slutz
@ 2012-09-01 15:57 ` Don Slutz
2012-09-02 10:52 ` Blue Swirl
2012-09-02 10:51 ` [Qemu-devel] [PATCH] CHECKPATCH Blue Swirl
` (5 subsequent siblings)
6 siblings, 1 reply; 14+ messages in thread
From: Don Slutz @ 2012-09-01 15:57 UTC (permalink / raw)
To: qemu-devel, blauwirbel; +Cc: Don Slutz
Also add more debug options to find this issue. They were not listed
in the help because the are not simple to understand the output of.
For an example:
WARNING: braces {} are necessary even for single statement blocks
+ } else
+ return env->regs[R_EAX];
total: 0 errors, 1 warnings, 41 lines checked
Signed-off-by: Don Slutz <Don@CloudSwitch.com>
---
scripts/checkpatch.pl | 72 ++++++++++++++++++++++++++++++++----------------
1 files changed, 48 insertions(+), 24 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index b98dc6c..140a3a6 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -97,6 +97,9 @@ my $dbg_values = 0;
my $dbg_possible = 0;
my $dbg_type = 0;
my $dbg_attr = 0;
+my $dbg_adv_dcs = 0;
+my $dbg_adv_apw = 0;
+my $dbg_adv_checking = 0;
for my $key (keys %debug) {
## no critic
eval "\${dbg_$key} = '$debug{$key}';";
@@ -2486,8 +2489,11 @@ sub process {
if ($line =~ /(^.*)\bif\b/ && $line !~ /\#\s*if/) {
my ($level, $endln, @chunks) =
ctx_statement_full($linenr, $realcnt, 1);
- #print "chunks<$#chunks> linenr<$linenr> endln<$endln> level<$level>\n";
- #print "APW: <<$chunks[1][0]>><<$chunks[1][1]>>\n";
+ if ($dbg_adv_apw) {
+ print "APW: chunks<$#chunks> linenr<$linenr> endln<$endln> level<$level>\n";
+ print "APW: <<$chunks[1][0]>><<$chunks[1][1]>>\n"
+ if $#chunks >= 1;
+ }
if ($#chunks >= 0 && $level == 0) {
my $allowed = 0;
my $seen = 0;
@@ -2500,7 +2506,8 @@ sub process {
my ($whitespace) = ($cond =~ /^((?:\s*\n[+-])*\s*)/s);
my $offset = statement_rawlines($whitespace) - 1;
- #print "COND<$cond> whitespace<$whitespace> offset<$offset>\n";
+ print "COND<$cond> whitespace<$whitespace> offset<$offset>\n"
+ if $dbg_adv_apw;
# We have looked at and allowed this specific line.
$suppress_ifbraces{$ln + $offset} = 1;
@@ -2512,18 +2519,22 @@ sub process {
$seen++ if ($block =~ /^\s*{/);
- #print "cond<$cond> block<$block> allowed<$allowed>\n";
+ print "APW: cond<$cond> block<$block> allowed<$allowed>\n"
+ if $dbg_adv_apw;
if (statement_lines($cond) > 1) {
- #print "APW: ALLOWED: cond<$cond>\n";
- $allowed = 1;
+ print "APW: ALLOWED: cond<$cond>\n"
+ if $dbg_adv_apw;
+ $allowed = 1;
}
if ($block =~/\b(?:if|for|while)\b/) {
- #print "APW: ALLOWED: block<$block>\n";
- $allowed = 1;
+ print "APW: ALLOWED: block<$block>\n"
+ if $dbg_adv_apw;
+ $allowed = 1;
}
if (statement_block_size($block) > 1) {
- #print "APW: ALLOWED: lines block<$block>\n";
- $allowed = 1;
+ print "APW: ALLOWED: lines block<$block>\n"
+ if $dbg_adv_apw;
+ $allowed = 1;
}
}
if ($seen != ($#chunks + 1)) {
@@ -2537,44 +2548,57 @@ sub process {
$line !~ /\#\s*else/) {
my $allowed = 0;
- # Check the pre-context.
- if (substr($line, 0, $-[0]) =~ /(\}\s*)$/) {
- #print "APW: ALLOWED: pre<$1>\n";
- $allowed = 1;
- }
+ # Check the pre-context.
+ if (substr($line, 0, $-[0]) =~ /(\}\s*)$/) {
+ my $pre = $1;
+
+ if ($line !~ /else/) {
+ print "APW: ALLOWED: pre<$pre> line<$line>\n"
+ if $dbg_adv_apw;
+ $allowed = 1;
+ }
+ }
my ($level, $endln, @chunks) =
ctx_statement_full($linenr, $realcnt, $-[0]);
# Check the condition.
my ($cond, $block) = @{$chunks[0]};
- #print "CHECKING<$linenr> cond<$cond> block<$block>\n";
+ print "CHECKING<$linenr> cond<$cond> block<$block>\n"
+ if $dbg_adv_checking;
if (defined $cond) {
substr($block, 0, length($cond), '');
}
if (statement_lines($cond) > 1) {
- #print "APW: ALLOWED: cond<$cond>\n";
- $allowed = 1;
+ print "APW: ALLOWED: cond<$cond>\n"
+ if $dbg_adv_apw;
+ $allowed = 1;
}
if ($block =~/\b(?:if|for|while)\b/) {
- #print "APW: ALLOWED: block<$block>\n";
- $allowed = 1;
+ print "APW: ALLOWED: block<$block>\n"
+ if $dbg_adv_apw;
+ $allowed = 1;
}
if (statement_block_size($block) > 1) {
- #print "APW: ALLOWED: lines block<$block>\n";
- $allowed = 1;
+ print "APW: ALLOWED: lines block<$block>\n"
+ if $dbg_adv_apw;
+ $allowed = 1;
}
# Check the post-context.
if (defined $chunks[1]) {
my ($cond, $block) = @{$chunks[1]};
+
if (defined $cond) {
substr($block, 0, length($cond), '');
}
if ($block =~ /^\s*\{/) {
- #print "APW: ALLOWED: chunk-1 block<$block>\n";
- $allowed = 1;
+ print "APW: ALLOWED: chunk-1 block<$block>\n"
+ if $dbg_adv_apw;
+ $allowed = 1;
}
}
+ print "DCS: level=$level block<$block> allowed=$allowed\n"
+ if $dbg_adv_dcs;
if ($level == 0 && $block !~ /^\s*\{/ && !$allowed) {
my $herectx = $here . "\n";;
my $cnt = statement_rawlines($block);
--
1.7.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [Qemu-devel] [PATCH] CHECKPATCH: Add warning for single else statement.
2012-09-01 15:57 ` [Qemu-devel] [PATCH] CHECKPATCH: Add warning for single else statement Don Slutz
@ 2012-09-02 10:52 ` Blue Swirl
0 siblings, 0 replies; 14+ messages in thread
From: Blue Swirl @ 2012-09-02 10:52 UTC (permalink / raw)
To: Don Slutz; +Cc: qemu-devel
On Sat, Sep 1, 2012 at 3:57 PM, Don Slutz <Don@cloudswitch.com> wrote:
> Also add more debug options to find this issue. They were not listed
> in the help because the are not simple to understand the output of.
These should form another patch, now it's not easy to see what was the
fix for 'else'.
>
> For an example:
>
> WARNING: braces {} are necessary even for single statement blocks
> + } else
> + return env->regs[R_EAX];
>
> total: 0 errors, 1 warnings, 41 lines checked
>
> Signed-off-by: Don Slutz <Don@CloudSwitch.com>
> ---
> scripts/checkpatch.pl | 72 ++++++++++++++++++++++++++++++++----------------
> 1 files changed, 48 insertions(+), 24 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index b98dc6c..140a3a6 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -97,6 +97,9 @@ my $dbg_values = 0;
> my $dbg_possible = 0;
> my $dbg_type = 0;
> my $dbg_attr = 0;
> +my $dbg_adv_dcs = 0;
> +my $dbg_adv_apw = 0;
> +my $dbg_adv_checking = 0;
> for my $key (keys %debug) {
> ## no critic
> eval "\${dbg_$key} = '$debug{$key}';";
> @@ -2486,8 +2489,11 @@ sub process {
> if ($line =~ /(^.*)\bif\b/ && $line !~ /\#\s*if/) {
> my ($level, $endln, @chunks) =
> ctx_statement_full($linenr, $realcnt, 1);
> - #print "chunks<$#chunks> linenr<$linenr> endln<$endln> level<$level>\n";
> - #print "APW: <<$chunks[1][0]>><<$chunks[1][1]>>\n";
> + if ($dbg_adv_apw) {
> + print "APW: chunks<$#chunks> linenr<$linenr> endln<$endln> level<$level>\n";
> + print "APW: <<$chunks[1][0]>><<$chunks[1][1]>>\n"
> + if $#chunks >= 1;
> + }
> if ($#chunks >= 0 && $level == 0) {
> my $allowed = 0;
> my $seen = 0;
> @@ -2500,7 +2506,8 @@ sub process {
> my ($whitespace) = ($cond =~ /^((?:\s*\n[+-])*\s*)/s);
> my $offset = statement_rawlines($whitespace) - 1;
>
> - #print "COND<$cond> whitespace<$whitespace> offset<$offset>\n";
> + print "COND<$cond> whitespace<$whitespace> offset<$offset>\n"
> + if $dbg_adv_apw;
>
> # We have looked at and allowed this specific line.
> $suppress_ifbraces{$ln + $offset} = 1;
> @@ -2512,18 +2519,22 @@ sub process {
>
> $seen++ if ($block =~ /^\s*{/);
>
> - #print "cond<$cond> block<$block> allowed<$allowed>\n";
> + print "APW: cond<$cond> block<$block> allowed<$allowed>\n"
> + if $dbg_adv_apw;
> if (statement_lines($cond) > 1) {
> - #print "APW: ALLOWED: cond<$cond>\n";
> - $allowed = 1;
> + print "APW: ALLOWED: cond<$cond>\n"
> + if $dbg_adv_apw;
> + $allowed = 1;
> }
> if ($block =~/\b(?:if|for|while)\b/) {
> - #print "APW: ALLOWED: block<$block>\n";
> - $allowed = 1;
> + print "APW: ALLOWED: block<$block>\n"
> + if $dbg_adv_apw;
> + $allowed = 1;
> }
> if (statement_block_size($block) > 1) {
> - #print "APW: ALLOWED: lines block<$block>\n";
> - $allowed = 1;
> + print "APW: ALLOWED: lines block<$block>\n"
> + if $dbg_adv_apw;
> + $allowed = 1;
> }
> }
> if ($seen != ($#chunks + 1)) {
> @@ -2537,44 +2548,57 @@ sub process {
> $line !~ /\#\s*else/) {
> my $allowed = 0;
>
> - # Check the pre-context.
> - if (substr($line, 0, $-[0]) =~ /(\}\s*)$/) {
> - #print "APW: ALLOWED: pre<$1>\n";
> - $allowed = 1;
> - }
> + # Check the pre-context.
> + if (substr($line, 0, $-[0]) =~ /(\}\s*)$/) {
> + my $pre = $1;
> +
> + if ($line !~ /else/) {
> + print "APW: ALLOWED: pre<$pre> line<$line>\n"
> + if $dbg_adv_apw;
> + $allowed = 1;
> + }
> + }
>
> my ($level, $endln, @chunks) =
> ctx_statement_full($linenr, $realcnt, $-[0]);
>
> # Check the condition.
> my ($cond, $block) = @{$chunks[0]};
> - #print "CHECKING<$linenr> cond<$cond> block<$block>\n";
> + print "CHECKING<$linenr> cond<$cond> block<$block>\n"
> + if $dbg_adv_checking;
> if (defined $cond) {
> substr($block, 0, length($cond), '');
> }
> if (statement_lines($cond) > 1) {
> - #print "APW: ALLOWED: cond<$cond>\n";
> - $allowed = 1;
> + print "APW: ALLOWED: cond<$cond>\n"
> + if $dbg_adv_apw;
> + $allowed = 1;
> }
> if ($block =~/\b(?:if|for|while)\b/) {
> - #print "APW: ALLOWED: block<$block>\n";
> - $allowed = 1;
> + print "APW: ALLOWED: block<$block>\n"
> + if $dbg_adv_apw;
> + $allowed = 1;
> }
> if (statement_block_size($block) > 1) {
> - #print "APW: ALLOWED: lines block<$block>\n";
> - $allowed = 1;
> + print "APW: ALLOWED: lines block<$block>\n"
> + if $dbg_adv_apw;
> + $allowed = 1;
> }
> # Check the post-context.
> if (defined $chunks[1]) {
> my ($cond, $block) = @{$chunks[1]};
> +
> if (defined $cond) {
> substr($block, 0, length($cond), '');
> }
> if ($block =~ /^\s*\{/) {
> - #print "APW: ALLOWED: chunk-1 block<$block>\n";
> - $allowed = 1;
> + print "APW: ALLOWED: chunk-1 block<$block>\n"
> + if $dbg_adv_apw;
> + $allowed = 1;
> }
> }
> + print "DCS: level=$level block<$block> allowed=$allowed\n"
> + if $dbg_adv_dcs;
> if ($level == 0 && $block !~ /^\s*\{/ && !$allowed) {
> my $herectx = $here . "\n";;
> my $cnt = statement_rawlines($block);
> --
> 1.7.1
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH] CHECKPATCH
2012-09-01 15:57 [Qemu-devel] [PATCH] CHECKPATCH Don Slutz
2012-09-01 15:57 ` [Qemu-devel] [PATCH] CHECKPATCH: Add warning for single else statement Don Slutz
@ 2012-09-02 10:51 ` Blue Swirl
2012-09-06 15:32 ` Avi Kivity
2012-09-02 23:22 ` [Qemu-devel] [PATCH v2 0/4] CHECKPATCH: Add warning for single else statement Don Slutz
` (4 subsequent siblings)
6 siblings, 1 reply; 14+ messages in thread
From: Blue Swirl @ 2012-09-02 10:51 UTC (permalink / raw)
To: Don Slutz; +Cc: qemu-devel
On Sat, Sep 1, 2012 at 3:57 PM, Don Slutz <Don@cloudswitch.com> wrote:
> I took my best guess as to how to handle tabs and long lines in
> checkpatch.pl itself.
Right, it was taken from Linux without much change.
>
> So I get:
> ...
> total: 0 errors, 7 warnings, 131 lines checked
>
> I also did not figure out how to run a more complete test of the
> change. Any pointers as to how would be helpful.
I've used the following snippet to check what happens with the last 100 commits:
for i in '' 1 2 3 4 5 6 7 8 9; do for j in 0 1 2 3 4 5 6 7 8 9; do
echo $i$j; git show HEAD~$i$j >/tmp/a; clear;head -20 /tmp/a;
./scripts/checkpatch.pl --root=. /tmp/a; read foo; done; done
Sadly, it also shows how much stuff gets committed without checking,
and on the other hand, the amount of false alarms.
>
>
> Don Slutz (1):
> CHECKPATCH: Add warning for single else statement.
>
> scripts/checkpatch.pl | 72 ++++++++++++++++++++++++++++++++----------------
> 1 files changed, 48 insertions(+), 24 deletions(-)
>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [Qemu-devel] [PATCH] CHECKPATCH
2012-09-02 10:51 ` [Qemu-devel] [PATCH] CHECKPATCH Blue Swirl
@ 2012-09-06 15:32 ` Avi Kivity
2012-09-08 7:33 ` Blue Swirl
0 siblings, 1 reply; 14+ messages in thread
From: Avi Kivity @ 2012-09-06 15:32 UTC (permalink / raw)
To: Blue Swirl; +Cc: Don Slutz, qemu-devel
On 09/02/2012 01:51 PM, Blue Swirl wrote:
> I've used the following snippet to check what happens with the last 100 commits:
> for i in '' 1 2 3 4 5 6 7 8 9; do for j in 0 1 2 3 4 5 6 7 8 9; do
> echo $i$j; git show HEAD~$i$j >/tmp/a; clear;head -20 /tmp/a;
> ./scripts/checkpatch.pl --root=. /tmp/a; read foo; done; done
>
> Sadly, it also shows how much stuff gets committed without checking,
> and on the other hand, the amount of false alarms.
Maintainers should add a checkpatch invocation as a git hook (advisory
only), this could reduce the amount of violations getting into the tree.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH] CHECKPATCH
2012-09-06 15:32 ` Avi Kivity
@ 2012-09-08 7:33 ` Blue Swirl
0 siblings, 0 replies; 14+ messages in thread
From: Blue Swirl @ 2012-09-08 7:33 UTC (permalink / raw)
To: Avi Kivity; +Cc: Don Slutz, qemu-devel
On Thu, Sep 6, 2012 at 3:32 PM, Avi Kivity <avi@redhat.com> wrote:
> On 09/02/2012 01:51 PM, Blue Swirl wrote:
>> I've used the following snippet to check what happens with the last 100 commits:
>> for i in '' 1 2 3 4 5 6 7 8 9; do for j in 0 1 2 3 4 5 6 7 8 9; do
>> echo $i$j; git show HEAD~$i$j >/tmp/a; clear;head -20 /tmp/a;
>> ./scripts/checkpatch.pl --root=. /tmp/a; read foo; done; done
>>
>> Sadly, it also shows how much stuff gets committed without checking,
>> and on the other hand, the amount of false alarms.
>
> Maintainers should add a checkpatch invocation as a git hook (advisory
> only), this could reduce the amount of violations getting into the tree.
I'd agree. But it looks like we have a few maintainers who actively
oppose CODING_STYLE and HACKING and don't reject patches with
violations or even inform the submitters about problems. The next
level is that we have committers who commit pulls without checking,
but I think the responsibility for checking patches for all possible
aspects (especially since there are real, deep, technical and
architectural issues to consider besides minor issues like style)
should not be duplicated at every level.
>
>
> --
> error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Qemu-devel] [PATCH v2 0/4] CHECKPATCH: Add warning for single else statement.
2012-09-01 15:57 [Qemu-devel] [PATCH] CHECKPATCH Don Slutz
2012-09-01 15:57 ` [Qemu-devel] [PATCH] CHECKPATCH: Add warning for single else statement Don Slutz
2012-09-02 10:51 ` [Qemu-devel] [PATCH] CHECKPATCH Blue Swirl
@ 2012-09-02 23:22 ` Don Slutz
2012-09-05 19:46 ` Blue Swirl
2012-09-02 23:22 ` [Qemu-devel] [PATCH v2 1/4] CHECKPATCH: Add --debug adv_dcs Don Slutz
` (3 subsequent siblings)
6 siblings, 1 reply; 14+ messages in thread
From: Don Slutz @ 2012-09-02 23:22 UTC (permalink / raw)
To: qemu-devel, blauwirbel; +Cc: Don Slutz
Also add more debug options to find this issue. They were not listed
in the help because the are not simple to understand the output of.
Using:
rm zz;
for k in 0 1 2 3; do
for i in 0 1 2 3 4 5 6 7 8 9; do
for j in 0 1 2 3 4 5 6 7 8 9; do
echo $k$i$j;
git show HEAD~$k$i$j >/tmp/a;
head -1 /tmp/a >>zz;
./scripts/checkpatch.pl --root=. /tmp/a >>zz;
done;
done;
done
With both the new and old versions that the same warnigns and errors were reported.
Don Slutz (4):
CHECKPATCH: Add --debug adv_dcs
CHECKPATCH: Add --debug adv_checking
CHECKPATCH: Add --debug adv_apw
CHECKPATCH: Add warning for single else statement.
scripts/checkpatch.pl | 68 ++++++++++++++++++++++++++++++++----------------
1 files changed, 45 insertions(+), 23 deletions(-)
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [Qemu-devel] [PATCH v2 0/4] CHECKPATCH: Add warning for single else statement.
2012-09-02 23:22 ` [Qemu-devel] [PATCH v2 0/4] CHECKPATCH: Add warning for single else statement Don Slutz
@ 2012-09-05 19:46 ` Blue Swirl
2012-09-06 10:32 ` Andreas Färber
0 siblings, 1 reply; 14+ messages in thread
From: Blue Swirl @ 2012-09-05 19:46 UTC (permalink / raw)
To: Don Slutz; +Cc: qemu-devel
Thanks, applied all.
On Sun, Sep 2, 2012 at 11:22 PM, Don Slutz <Don@cloudswitch.com> wrote:
> Also add more debug options to find this issue. They were not listed
> in the help because the are not simple to understand the output of.
>
> Using:
>
> rm zz;
> for k in 0 1 2 3; do
> for i in 0 1 2 3 4 5 6 7 8 9; do
> for j in 0 1 2 3 4 5 6 7 8 9; do
> echo $k$i$j;
> git show HEAD~$k$i$j >/tmp/a;
> head -1 /tmp/a >>zz;
> ./scripts/checkpatch.pl --root=. /tmp/a >>zz;
> done;
> done;
> done
>
> With both the new and old versions that the same warnigns and errors were reported.
>
> Don Slutz (4):
> CHECKPATCH: Add --debug adv_dcs
> CHECKPATCH: Add --debug adv_checking
> CHECKPATCH: Add --debug adv_apw
> CHECKPATCH: Add warning for single else statement.
>
> scripts/checkpatch.pl | 68 ++++++++++++++++++++++++++++++++----------------
> 1 files changed, 45 insertions(+), 23 deletions(-)
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/4] CHECKPATCH: Add warning for single else statement.
2012-09-05 19:46 ` Blue Swirl
@ 2012-09-06 10:32 ` Andreas Färber
2012-09-08 7:08 ` Blue Swirl
0 siblings, 1 reply; 14+ messages in thread
From: Andreas Färber @ 2012-09-06 10:32 UTC (permalink / raw)
To: Blue Swirl; +Cc: Don Slutz, qemu-devel
Am 05.09.2012 21:46, schrieb Blue Swirl:
> Thanks, applied all.
Question: Are additions to checkpatch.pl supposed to be in QEMU Coding
Style as done here? Do you plan to convert it consistently then?
checkpatch.pl uses tabs consistently, making checkpatch.pl complain. ;)
The alternative would be to stick to upstream formatting and exempt
checkpatch.pl from its own checks.
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/4] CHECKPATCH: Add warning for single else statement.
2012-09-06 10:32 ` Andreas Färber
@ 2012-09-08 7:08 ` Blue Swirl
0 siblings, 0 replies; 14+ messages in thread
From: Blue Swirl @ 2012-09-08 7:08 UTC (permalink / raw)
To: Andreas Färber; +Cc: Don Slutz, qemu-devel
On Thu, Sep 6, 2012 at 10:32 AM, Andreas Färber <afaerber@suse.de> wrote:
> Am 05.09.2012 21:46, schrieb Blue Swirl:
>> Thanks, applied all.
>
> Question: Are additions to checkpatch.pl supposed to be in QEMU Coding
> Style as done here? Do you plan to convert it consistently then?
> checkpatch.pl uses tabs consistently, making checkpatch.pl complain. ;)
> The alternative would be to stick to upstream formatting and exempt
> checkpatch.pl from its own checks.
I'd suppose checkpatch.pl can't check Perl, Python or sh syntax so we
could try to limit some of the checks to C files only. For example
tabs are mandatory as the initial character for Makefile rules, we
could in theory demand spaces otherwise but I guess nobody cares
enough to add checks for this.
>
> Andreas
>
> --
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Qemu-devel] [PATCH v2 1/4] CHECKPATCH: Add --debug adv_dcs
2012-09-01 15:57 [Qemu-devel] [PATCH] CHECKPATCH Don Slutz
` (2 preceding siblings ...)
2012-09-02 23:22 ` [Qemu-devel] [PATCH v2 0/4] CHECKPATCH: Add warning for single else statement Don Slutz
@ 2012-09-02 23:22 ` Don Slutz
2012-09-02 23:22 ` [Qemu-devel] [PATCH v2 2/4] CHECKPATCH: Add --debug adv_checking Don Slutz
` (2 subsequent siblings)
6 siblings, 0 replies; 14+ messages in thread
From: Don Slutz @ 2012-09-02 23:22 UTC (permalink / raw)
To: qemu-devel, blauwirbel; +Cc: Don Slutz
Add debug options to find this issue. They were not listed
in the help because the are not simple to understand the output of.
Signed-off-by: Don Slutz <Don@CloudSwitch.com>
---
scripts/checkpatch.pl | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index b98dc6c..0b0f3f3 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -97,6 +97,7 @@ my $dbg_values = 0;
my $dbg_possible = 0;
my $dbg_type = 0;
my $dbg_attr = 0;
+my $dbg_adv_dcs = 0;
for my $key (keys %debug) {
## no critic
eval "\${dbg_$key} = '$debug{$key}';";
@@ -2575,6 +2576,8 @@ sub process {
$allowed = 1;
}
}
+ print "DCS: level=$level block<$block> allowed=$allowed\n"
+ if $dbg_adv_dcs;
if ($level == 0 && $block !~ /^\s*\{/ && !$allowed) {
my $herectx = $here . "\n";;
my $cnt = statement_rawlines($block);
--
1.7.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PATCH v2 2/4] CHECKPATCH: Add --debug adv_checking
2012-09-01 15:57 [Qemu-devel] [PATCH] CHECKPATCH Don Slutz
` (3 preceding siblings ...)
2012-09-02 23:22 ` [Qemu-devel] [PATCH v2 1/4] CHECKPATCH: Add --debug adv_dcs Don Slutz
@ 2012-09-02 23:22 ` Don Slutz
2012-09-02 23:22 ` [Qemu-devel] [PATCH v2 3/4] CHECKPATCH: Add --debug adv_apw Don Slutz
2012-09-02 23:22 ` [Qemu-devel] [PATCH v2 4/4] CHECKPATCH: Add warning for single else statement Don Slutz
6 siblings, 0 replies; 14+ messages in thread
From: Don Slutz @ 2012-09-02 23:22 UTC (permalink / raw)
To: qemu-devel, blauwirbel; +Cc: Don Slutz
Add debug options to find this issue. They were not listed
in the help because the are not simple to understand the output of.
Signed-off-by: Don Slutz <Don@CloudSwitch.com>
---
scripts/checkpatch.pl | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 0b0f3f3..8c83b56 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -98,6 +98,7 @@ my $dbg_possible = 0;
my $dbg_type = 0;
my $dbg_attr = 0;
my $dbg_adv_dcs = 0;
+my $dbg_adv_checking = 0;
for my $key (keys %debug) {
## no critic
eval "\${dbg_$key} = '$debug{$key}';";
@@ -2549,7 +2550,8 @@ sub process {
# Check the condition.
my ($cond, $block) = @{$chunks[0]};
- #print "CHECKING<$linenr> cond<$cond> block<$block>\n";
+ print "CHECKING<$linenr> cond<$cond> block<$block>\n"
+ if $dbg_adv_checking;
if (defined $cond) {
substr($block, 0, length($cond), '');
}
--
1.7.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PATCH v2 3/4] CHECKPATCH: Add --debug adv_apw
2012-09-01 15:57 [Qemu-devel] [PATCH] CHECKPATCH Don Slutz
` (4 preceding siblings ...)
2012-09-02 23:22 ` [Qemu-devel] [PATCH v2 2/4] CHECKPATCH: Add --debug adv_checking Don Slutz
@ 2012-09-02 23:22 ` Don Slutz
2012-09-02 23:22 ` [Qemu-devel] [PATCH v2 4/4] CHECKPATCH: Add warning for single else statement Don Slutz
6 siblings, 0 replies; 14+ messages in thread
From: Don Slutz @ 2012-09-02 23:22 UTC (permalink / raw)
To: qemu-devel, blauwirbel; +Cc: Don Slutz
Add debug options to find this issue. They were not listed
in the help because the are not simple to understand the output of.
Signed-off-by: Don Slutz <Don@CloudSwitch.com>
---
scripts/checkpatch.pl | 51 ++++++++++++++++++++++++++++++------------------
1 files changed, 32 insertions(+), 19 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 8c83b56..7ec8846 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -99,6 +99,7 @@ my $dbg_type = 0;
my $dbg_attr = 0;
my $dbg_adv_dcs = 0;
my $dbg_adv_checking = 0;
+my $dbg_adv_apw = 0;
for my $key (keys %debug) {
## no critic
eval "\${dbg_$key} = '$debug{$key}';";
@@ -2488,8 +2489,11 @@ sub process {
if ($line =~ /(^.*)\bif\b/ && $line !~ /\#\s*if/) {
my ($level, $endln, @chunks) =
ctx_statement_full($linenr, $realcnt, 1);
- #print "chunks<$#chunks> linenr<$linenr> endln<$endln> level<$level>\n";
- #print "APW: <<$chunks[1][0]>><<$chunks[1][1]>>\n";
+ if ($dbg_adv_apw) {
+ print "APW: chunks<$#chunks> linenr<$linenr> endln<$endln> level<$level>\n";
+ print "APW: <<$chunks[1][0]>><<$chunks[1][1]>>\n"
+ if $#chunks >= 1;
+ }
if ($#chunks >= 0 && $level == 0) {
my $allowed = 0;
my $seen = 0;
@@ -2514,18 +2518,22 @@ sub process {
$seen++ if ($block =~ /^\s*{/);
- #print "cond<$cond> block<$block> allowed<$allowed>\n";
+ print "APW: cond<$cond> block<$block> allowed<$allowed>\n"
+ if $dbg_adv_apw;
if (statement_lines($cond) > 1) {
- #print "APW: ALLOWED: cond<$cond>\n";
- $allowed = 1;
+ print "APW: ALLOWED: cond<$cond>\n"
+ if $dbg_adv_apw;
+ $allowed = 1;
}
if ($block =~/\b(?:if|for|while)\b/) {
- #print "APW: ALLOWED: block<$block>\n";
- $allowed = 1;
+ print "APW: ALLOWED: block<$block>\n"
+ if $dbg_adv_apw;
+ $allowed = 1;
}
if (statement_block_size($block) > 1) {
- #print "APW: ALLOWED: lines block<$block>\n";
- $allowed = 1;
+ print "APW: ALLOWED: lines block<$block>\n"
+ if $dbg_adv_apw;
+ $allowed = 1;
}
}
if ($seen != ($#chunks + 1)) {
@@ -2541,8 +2549,9 @@ sub process {
# Check the pre-context.
if (substr($line, 0, $-[0]) =~ /(\}\s*)$/) {
- #print "APW: ALLOWED: pre<$1>\n";
- $allowed = 1;
+ print "APW: ALLOWED: pre<$pre> line<$line>\n"
+ if $dbg_adv_apw;
+ $allowed = 1;
}
my ($level, $endln, @chunks) =
@@ -2556,16 +2565,19 @@ sub process {
substr($block, 0, length($cond), '');
}
if (statement_lines($cond) > 1) {
- #print "APW: ALLOWED: cond<$cond>\n";
- $allowed = 1;
+ print "APW: ALLOWED: cond<$cond>\n"
+ if $dbg_adv_apw;
+ $allowed = 1;
}
if ($block =~/\b(?:if|for|while)\b/) {
- #print "APW: ALLOWED: block<$block>\n";
- $allowed = 1;
+ print "APW: ALLOWED: block<$block>\n"
+ if $dbg_adv_apw;
+ $allowed = 1;
}
if (statement_block_size($block) > 1) {
- #print "APW: ALLOWED: lines block<$block>\n";
- $allowed = 1;
+ print "APW: ALLOWED: lines block<$block>\n"
+ if $dbg_adv_apw;
+ $allowed = 1;
}
# Check the post-context.
if (defined $chunks[1]) {
@@ -2574,8 +2586,9 @@ sub process {
substr($block, 0, length($cond), '');
}
if ($block =~ /^\s*\{/) {
- #print "APW: ALLOWED: chunk-1 block<$block>\n";
- $allowed = 1;
+ print "APW: ALLOWED: chunk-1 block<$block>\n"
+ if $dbg_adv_apw;
+ $allowed = 1;
}
}
print "DCS: level=$level block<$block> allowed=$allowed\n"
--
1.7.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PATCH v2 4/4] CHECKPATCH: Add warning for single else statement.
2012-09-01 15:57 [Qemu-devel] [PATCH] CHECKPATCH Don Slutz
` (5 preceding siblings ...)
2012-09-02 23:22 ` [Qemu-devel] [PATCH v2 3/4] CHECKPATCH: Add --debug adv_apw Don Slutz
@ 2012-09-02 23:22 ` Don Slutz
6 siblings, 0 replies; 14+ messages in thread
From: Don Slutz @ 2012-09-02 23:22 UTC (permalink / raw)
To: qemu-devel, blauwirbel; +Cc: Don Slutz
For an example:
WARNING: braces {} are necessary even for single statement blocks
+ } else
+ return env->regs[R_EAX];
total: 0 errors, 1 warnings, 41 lines checked
Signed-off-by: Don Slutz <Don@CloudSwitch.com>
---
scripts/checkpatch.pl | 16 ++++++++++------
1 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 7ec8846..ec0aa4c 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2547,12 +2547,16 @@ sub process {
$line !~ /\#\s*else/) {
my $allowed = 0;
- # Check the pre-context.
- if (substr($line, 0, $-[0]) =~ /(\}\s*)$/) {
- print "APW: ALLOWED: pre<$pre> line<$line>\n"
- if $dbg_adv_apw;
- $allowed = 1;
- }
+ # Check the pre-context.
+ if (substr($line, 0, $-[0]) =~ /(\}\s*)$/) {
+ my $pre = $1;
+
+ if ($line !~ /else/) {
+ print "APW: ALLOWED: pre<$pre> line<$line>\n"
+ if $dbg_adv_apw;
+ $allowed = 1;
+ }
+ }
my ($level, $endln, @chunks) =
ctx_statement_full($linenr, $realcnt, $-[0]);
--
1.7.1
^ permalink raw reply related [flat|nested] 14+ messages in thread