* [Qemu-devel] [PATCH] checkpatch: warn when using volatile with a comment
@ 2017-12-15 18:18 Marc-André Lureau
2017-12-15 18:39 ` no-reply
2017-12-18 12:49 ` Paolo Bonzini
0 siblings, 2 replies; 7+ messages in thread
From: Marc-André Lureau @ 2017-12-15 18:18 UTC (permalink / raw)
To: qemu-devel; +Cc: famz, pbonzini, lersek, Marc-André Lureau
Instead of an error, lower to a warning message, assuming the comment
gives some justification.
Discussed in:
'[Qemu-devel] [PATCH] dump-guest-memory.py: fix "You can't do that without a process to debug"'
Suggested-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
scripts/checkpatch.pl | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 34df753571..f5a523af10 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2476,7 +2476,12 @@ sub process {
# no volatiles please
my $asm_volatile = qr{\b(__asm__|asm)\s+(__volatile__|volatile)\b};
if ($line =~ /\bvolatile\b/ && $line !~ /$asm_volatile/) {
- ERROR("Use of volatile is usually wrong: see Documentation/volatile-considered-harmful.txt\n" . $herecurr);
+ my $msg = "Use of volatile is usually wrong: see Documentation/volatile-considered-harmful.txt\n" . $herecurr;
+ if (ctx_has_comment($first_line, $linenr)) {
+ WARN($msg);
+ } else {
+ ERROR($msg);
+ }
}
# warn about #if 0
--
2.15.1.355.g36791d7216
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] checkpatch: warn when using volatile with a comment
2017-12-15 18:18 [Qemu-devel] [PATCH] checkpatch: warn when using volatile with a comment Marc-André Lureau
@ 2017-12-15 18:39 ` no-reply
2017-12-18 12:49 ` Paolo Bonzini
1 sibling, 0 replies; 7+ messages in thread
From: no-reply @ 2017-12-15 18:39 UTC (permalink / raw)
To: marcandre.lureau; +Cc: famz, qemu-devel, pbonzini
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 20171215181810.4122-1-marcandre.lureau@redhat.com
Subject: [Qemu-devel] [PATCH] checkpatch: warn when using volatile with a comment
=== 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
From https://github.com/patchew-project/qemu
t [tag update] patchew/20171215150659.1811-1-marcandre.lureau@redhat.com -> patchew/20171215150659.1811-1-marcandre.lureau@redhat.com
t [tag update] patchew/20171215181810.4122-1-marcandre.lureau@redhat.com -> patchew/20171215181810.4122-1-marcandre.lureau@redhat.com
Switched to a new branch 'test'
80828ed1b5 checkpatch: warn when using volatile with a comment
=== OUTPUT BEGIN ===
Checking PATCH 1/1: checkpatch: warn when using volatile with a comment...
ERROR: line over 90 characters
#28: FILE: scripts/checkpatch.pl:2479:
+ my $msg = "Use of volatile is usually wrong: see Documentation/volatile-considered-harmful.txt\n" . $herecurr;
total: 1 errors, 0 warnings, 13 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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] checkpatch: warn when using volatile with a comment
2017-12-15 18:18 [Qemu-devel] [PATCH] checkpatch: warn when using volatile with a comment Marc-André Lureau
2017-12-15 18:39 ` no-reply
@ 2017-12-18 12:49 ` Paolo Bonzini
2017-12-18 12:54 ` Marc-André Lureau
2017-12-18 13:36 ` Darren Kenny
1 sibling, 2 replies; 7+ messages in thread
From: Paolo Bonzini @ 2017-12-18 12:49 UTC (permalink / raw)
To: Marc-André Lureau, qemu-devel; +Cc: famz, lersek
On 15/12/2017 19:18, Marc-André Lureau wrote:
> Instead of an error, lower to a warning message, assuming the comment
> gives some justification.
>
> Discussed in:
> '[Qemu-devel] [PATCH] dump-guest-memory.py: fix "You can't do that without a process to debug"'
>
> Suggested-by: Fam Zheng <famz@redhat.com>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
We can drop the error at all if there is a comment. Also, "volatile sig_atomic_t" is probably self-explanatory and usually correct. So what about this:
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index f5a523af10..3dc27d9656 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2475,13 +2475,11 @@ sub process {
# no volatiles please
my $asm_volatile = qr{\b(__asm__|asm)\s+(__volatile__|volatile)\b};
- if ($line =~ /\bvolatile\b/ && $line !~ /$asm_volatile/) {
- my $msg = "Use of volatile is usually wrong: see Documentation/volatile-considered-harmful.txt\n" . $herecurr;
- if (ctx_has_comment($first_line, $linenr)) {
- WARN($msg);
- } else {
- ERROR($msg);
- }
+ if ($line =~ /\bvolatile\b/ && $line !~ /$asm_volatile/ &&
+ $line !~ /sig_atomic_t/ &&
+ !ctx_has_comment($first_line, $linenr)) {
+ my $msg = "Use of volatile is usually wrong, please add a comment\n" . $herecurr;
+ ERROR($msg);
}
# warn about #if 0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] checkpatch: warn when using volatile with a comment
2017-12-18 12:49 ` Paolo Bonzini
@ 2017-12-18 12:54 ` Marc-André Lureau
2017-12-18 13:02 ` Paolo Bonzini
2017-12-18 13:36 ` Darren Kenny
1 sibling, 1 reply; 7+ messages in thread
From: Marc-André Lureau @ 2017-12-18 12:54 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: QEMU, Laszlo Ersek, Fam Zheng
Hi
On Mon, Dec 18, 2017 at 1:49 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 15/12/2017 19:18, Marc-André Lureau wrote:
>> Instead of an error, lower to a warning message, assuming the comment
>> gives some justification.
>>
>> Discussed in:
>> '[Qemu-devel] [PATCH] dump-guest-memory.py: fix "You can't do that without a process to debug"'
>>
>> Suggested-by: Fam Zheng <famz@redhat.com>
>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> We can drop the error at all if there is a comment. Also, "volatile sig_atomic_t" is probably self-explanatory and usually correct. So what about this:
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index f5a523af10..3dc27d9656 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -2475,13 +2475,11 @@ sub process {
>
> # no volatiles please
> my $asm_volatile = qr{\b(__asm__|asm)\s+(__volatile__|volatile)\b};
> - if ($line =~ /\bvolatile\b/ && $line !~ /$asm_volatile/) {
> - my $msg = "Use of volatile is usually wrong: see Documentation/volatile-considered-harmful.txt\n" . $herecurr;
> - if (ctx_has_comment($first_line, $linenr)) {
> - WARN($msg);
> - } else {
> - ERROR($msg);
> - }
> + if ($line =~ /\bvolatile\b/ && $line !~ /$asm_volatile/ &&
> + $line !~ /sig_atomic_t/ &&
> + !ctx_has_comment($first_line, $linenr)) {
> + my $msg = "Use of volatile is usually wrong, please add a comment\n" . $herecurr;
> + ERROR($msg);
> }
>
Fine for me, can you send a proper patch?
Thanks
--
Marc-André Lureau
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] checkpatch: warn when using volatile with a comment
2017-12-18 12:54 ` Marc-André Lureau
@ 2017-12-18 13:02 ` Paolo Bonzini
0 siblings, 0 replies; 7+ messages in thread
From: Paolo Bonzini @ 2017-12-18 13:02 UTC (permalink / raw)
To: Marc-André Lureau; +Cc: QEMU, Laszlo Ersek, Fam Zheng
On 18/12/2017 13:54, Marc-André Lureau wrote:
> Hi
>
> On Mon, Dec 18, 2017 at 1:49 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> On 15/12/2017 19:18, Marc-André Lureau wrote:
>>> Instead of an error, lower to a warning message, assuming the comment
>>> gives some justification.
>>>
>>> Discussed in:
>>> '[Qemu-devel] [PATCH] dump-guest-memory.py: fix "You can't do that without a process to debug"'
>>>
>>> Suggested-by: Fam Zheng <famz@redhat.com>
>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>
>> We can drop the error at all if there is a comment. Also, "volatile sig_atomic_t" is probably self-explanatory and usually correct. So what about this:
>>
>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>> index f5a523af10..3dc27d9656 100755
>> --- a/scripts/checkpatch.pl
>> +++ b/scripts/checkpatch.pl
>> @@ -2475,13 +2475,11 @@ sub process {
>>
>> # no volatiles please
>> my $asm_volatile = qr{\b(__asm__|asm)\s+(__volatile__|volatile)\b};
>> - if ($line =~ /\bvolatile\b/ && $line !~ /$asm_volatile/) {
>> - my $msg = "Use of volatile is usually wrong: see Documentation/volatile-considered-harmful.txt\n" . $herecurr;
>> - if (ctx_has_comment($first_line, $linenr)) {
>> - WARN($msg);
>> - } else {
>> - ERROR($msg);
>> - }
>> + if ($line =~ /\bvolatile\b/ && $line !~ /$asm_volatile/ &&
>> + $line !~ /sig_atomic_t/ &&
>> + !ctx_has_comment($first_line, $linenr)) {
>> + my $msg = "Use of volatile is usually wrong, please add a comment\n" . $herecurr;
>> + ERROR($msg);
>> }
>>
>
> Fine for me, can you send a proper patch?
Yes, will do.
Paolo
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] checkpatch: warn when using volatile with a comment
2017-12-18 12:49 ` Paolo Bonzini
2017-12-18 12:54 ` Marc-André Lureau
@ 2017-12-18 13:36 ` Darren Kenny
2017-12-18 13:38 ` Darren Kenny
1 sibling, 1 reply; 7+ messages in thread
From: Darren Kenny @ 2017-12-18 13:36 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Marc-André Lureau, qemu-devel, lersek, famz
Hi Paolo,
Slight nit on the subject line, did you mean to s/with/without/ -
that seems to reflect the change in the patch more correctly.
Thanks,
Darren.
On Mon, Dec 18, 2017 at 01:49:52PM +0100, Paolo Bonzini wrote:
>On 15/12/2017 19:18, Marc-André Lureau wrote:
>> Instead of an error, lower to a warning message, assuming the comment
>> gives some justification.
>>
>> Discussed in:
>> '[Qemu-devel] [PATCH] dump-guest-memory.py: fix "You can't do that without a process to debug"'
>>
>> Suggested-by: Fam Zheng <famz@redhat.com>
>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>
>We can drop the error at all if there is a comment. Also, "volatile sig_atomic_t" is probably self-explanatory and usually correct. So what about this:
>
>diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>index f5a523af10..3dc27d9656 100755
>--- a/scripts/checkpatch.pl
>+++ b/scripts/checkpatch.pl
>@@ -2475,13 +2475,11 @@ sub process {
>
> # no volatiles please
> my $asm_volatile = qr{\b(__asm__|asm)\s+(__volatile__|volatile)\b};
>- if ($line =~ /\bvolatile\b/ && $line !~ /$asm_volatile/) {
>- my $msg = "Use of volatile is usually wrong: see Documentation/volatile-considered-harmful.txt\n" . $herecurr;
>- if (ctx_has_comment($first_line, $linenr)) {
>- WARN($msg);
>- } else {
>- ERROR($msg);
>- }
>+ if ($line =~ /\bvolatile\b/ && $line !~ /$asm_volatile/ &&
>+ $line !~ /sig_atomic_t/ &&
>+ !ctx_has_comment($first_line, $linenr)) {
>+ my $msg = "Use of volatile is usually wrong, please add a comment\n" . $herecurr;
>+ ERROR($msg);
> }
>
> # warn about #if 0
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] checkpatch: warn when using volatile with a comment
2017-12-18 13:36 ` Darren Kenny
@ 2017-12-18 13:38 ` Darren Kenny
0 siblings, 0 replies; 7+ messages in thread
From: Darren Kenny @ 2017-12-18 13:38 UTC (permalink / raw)
To: Paolo Bonzini, Marc-André Lureau, qemu-devel, lersek, famz
Nevermind, saw that updated comment in the later patch...
Thanks,
Darren.
On Mon, Dec 18, 2017 at 01:36:52PM +0000, Darren Kenny wrote:
>Hi Paolo,
>
>Slight nit on the subject line, did you mean to s/with/without/ -
>that seems to reflect the change in the patch more correctly.
>
>Thanks,
>
>Darren.
>
>On Mon, Dec 18, 2017 at 01:49:52PM +0100, Paolo Bonzini wrote:
>>On 15/12/2017 19:18, Marc-André Lureau wrote:
>>>Instead of an error, lower to a warning message, assuming the comment
>>>gives some justification.
>>>
>>>Discussed in:
>>>'[Qemu-devel] [PATCH] dump-guest-memory.py: fix "You can't do that without a process to debug"'
>>>
>>>Suggested-by: Fam Zheng <famz@redhat.com>
>>>Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>
>>We can drop the error at all if there is a comment. Also, "volatile sig_atomic_t" is probably self-explanatory and usually correct. So what about this:
>>
>>diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>>index f5a523af10..3dc27d9656 100755
>>--- a/scripts/checkpatch.pl
>>+++ b/scripts/checkpatch.pl
>>@@ -2475,13 +2475,11 @@ sub process {
>>
>># no volatiles please
>> my $asm_volatile = qr{\b(__asm__|asm)\s+(__volatile__|volatile)\b};
>>- if ($line =~ /\bvolatile\b/ && $line !~ /$asm_volatile/) {
>>- my $msg = "Use of volatile is usually wrong: see Documentation/volatile-considered-harmful.txt\n" . $herecurr;
>>- if (ctx_has_comment($first_line, $linenr)) {
>>- WARN($msg);
>>- } else {
>>- ERROR($msg);
>>- }
>>+ if ($line =~ /\bvolatile\b/ && $line !~ /$asm_volatile/ &&
>>+ $line !~ /sig_atomic_t/ &&
>>+ !ctx_has_comment($first_line, $linenr)) {
>>+ my $msg = "Use of volatile is usually wrong, please add a comment\n" . $herecurr;
>>+ ERROR($msg);
>> }
>>
>># warn about #if 0
>>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-12-18 13:38 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-15 18:18 [Qemu-devel] [PATCH] checkpatch: warn when using volatile with a comment Marc-André Lureau
2017-12-15 18:39 ` no-reply
2017-12-18 12:49 ` Paolo Bonzini
2017-12-18 12:54 ` Marc-André Lureau
2017-12-18 13:02 ` Paolo Bonzini
2017-12-18 13:36 ` Darren Kenny
2017-12-18 13:38 ` Darren Kenny
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.