[Qemu-devel] [PATCH] checkpatch: replace ERROR with WARN for extern checking.

Jiang Biao posted 1 patch 6 years, 5 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/1507696406-11168-1-git-send-email-jiang.biao2@zte.com.cn
Test checkpatch failed
Test docker passed
Test s390x passed
scripts/checkpatch.pl | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
[Qemu-devel] [PATCH] checkpatch: replace ERROR with WARN for extern checking.
Posted by Jiang Biao 6 years, 5 months ago
There are some rare cases which need external declarations in .c
files. patchew.org and checkpatch.pl will complain errors on
patches for these declarations.

Degrade ERROR to WARN to erase the error complaints taking
checkpatch.pl in kernel as reference.

Signed-off-by: Jiang Biao <jiang.biao2@zte.com.cn>
---
 scripts/checkpatch.pl | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 3c0a28e..9123788 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2546,7 +2546,7 @@ sub process {
 			if ($s =~ /^\s*;/ &&
 			    $function_name ne 'uninitialized_var')
 			{
-				ERROR("externs should be avoided in .c files\n" .  $herecurr);
+				WARN("externs should be avoided in .c files\n" .  $herecurr);
 			}
 
 			if ($paren_space =~ /\n/) {
@@ -2556,7 +2556,7 @@ sub process {
 		} elsif ($realfile =~ /\.c$/ && defined $stat &&
 		    $stat =~ /^.\s*extern\s+/)
 		{
-			ERROR("externs should be avoided in .c files\n" .  $herecurr);
+			WARN("externs should be avoided in .c files\n" .  $herecurr);
 		}
 
 # check for pointless casting of g_malloc return
-- 
2.9.5


Re: [Qemu-devel] [PATCH] checkpatch: replace ERROR with WARN for extern checking.
Posted by no-reply@patchew.org 6 years, 5 months ago
Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 1507696406-11168-1-git-send-email-jiang.biao2@zte.com.cn
Subject: [Qemu-devel] [PATCH] checkpatch: replace ERROR with WARN for extern checking.

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
5a8e815e78 checkpatch: replace ERROR with WARN for extern checking.

=== OUTPUT BEGIN ===
Checking PATCH 1/1: checkpatch: replace ERROR with WARN for extern checking....
ERROR: line over 90 characters
#25: FILE: scripts/checkpatch.pl:2550:
+				WARN("externs should be avoided in .c files\n" .  $herecurr);

WARNING: line over 80 characters
#34: FILE: scripts/checkpatch.pl:2560:
+			WARN("externs should be avoided in .c files\n" .  $herecurr);

total: 1 errors, 1 warnings, 16 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org
Re: [Qemu-devel] [PATCH] checkpatch: replace ERROR with WARN for extern checking.
Posted by Peter Maydell 6 years, 5 months ago
On 11 October 2017 at 05:33, Jiang Biao <jiang.biao2@zte.com.cn> wrote:
> There are some rare cases which need external declarations in .c
> files. patchew.org and checkpatch.pl will complain errors on
> patches for these declarations.
>
> Degrade ERROR to WARN to erase the error complaints taking
> checkpatch.pl in kernel as reference.
>
> Signed-off-by: Jiang Biao <jiang.biao2@zte.com.cn>
> ---
>  scripts/checkpatch.pl | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

I'd rather not drop this to a warning for the sake of a single
use case that's already in the tree (and which if you really
cared about you could work around by putting the link_error()
declaration in a header file I suppose, though I wouldn't
bother personally.)

thanks
-- PMM

Re: [Qemu-devel][PATCH] checkpatch: replace ERROR with WARN forextern checking.
Posted by jiang.biao2@zte.com.cn 6 years, 5 months ago
PiBPbiAxMSBPY3RvYmVyIDIwMTcgYXQgMDU6MzMsIEppYW5nIEJpYW8gPGppYW5nLmJpYW8yQHp0
ZS5jb20uY24+IHdyb3RlOg0KPiANCj4gSSdkIHJhdGhlciBub3QgZHJvcCB0aGlzIHRvIGEgd2Fy
bmluZyBmb3IgdGhlIHNha2Ugb2YgYSBzaW5nbGUNCj4gdXNlIGNhc2UgdGhhdCdzIGFscmVhZHkg
aW4gdGhlIHRyZWUgKGFuZCB3aGljaCBpZiB5b3UgcmVhbGx5DQo+IGNhcmVkIGFib3V0IHlvdSBj
b3VsZCB3b3JrIGFyb3VuZCBieSBwdXR0aW5nIHRoZSBsaW5rX2Vycm9yKCkNCj4gZGVjbGFyYXRp
b24gaW4gYSBoZWFkZXIgZmlsZSBJIHN1cHBvc2UsIHRob3VnaCBJIHdvdWxkbid0DQo+IGJvdGhl
ciBwZXJzb25hbGx5LikNCg0KDQoNCk5laXRoZXIgd291bGQgSSA6KS4gVGhhbmtzIGZvciB0aGUg
cmVwbHkuIA0KDQoNClJlZ2FyZHMsIEppYW5n

Re: [Qemu-devel] [PATCH] checkpatch: replace ERROR with WARN for extern checking.
Posted by Markus Armbruster 6 years, 4 months ago
Peter Maydell <peter.maydell@linaro.org> writes:

> On 11 October 2017 at 05:33, Jiang Biao <jiang.biao2@zte.com.cn> wrote:
>> There are some rare cases which need external declarations in .c
>> files. patchew.org and checkpatch.pl will complain errors on
>> patches for these declarations.
>>
>> Degrade ERROR to WARN to erase the error complaints taking
>> checkpatch.pl in kernel as reference.
>>
>> Signed-off-by: Jiang Biao <jiang.biao2@zte.com.cn>
>> ---
>>  scripts/checkpatch.pl | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> I'd rather not drop this to a warning for the sake of a single
> use case that's already in the tree (and which if you really

Concur.  Rare false positives from checkpatch are tolerable.

> cared about you could work around by putting the link_error()
> declaration in a header file I suppose, though I wouldn't
> bother personally.)