* [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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).