* [PATCH] checkpatch: Detect '%#' or '%0#' in printf-style format strings
@ 2020-09-14 6:01 Dov Murik
2020-09-14 6:55 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 5+ messages in thread
From: Dov Murik @ 2020-09-14 6:01 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel; +Cc: Dov Murik
According to the coding style document, we should use literal '0x' prefix
instead of printf's '#' flag (which appears as '%#' or '%0#' in the format
string). Add a checkpatch rule to enforce that.
Note that checkpatch already had a similar rule for trace-events files.
Example usage:
$ scripts/checkpatch.pl --file chardev/baum.c
...
ERROR: Don't use '#' flag of printf format ('%#') in format strings, use '0x' prefix instead
#366: FILE: chardev/baum.c:366:
+ DPRINTF("Broken packet %#2x, tossing\n", req); \
...
ERROR: Don't use '#' flag of printf format ('%#') in format strings, use '0x' prefix instead
#472: FILE: chardev/baum.c:472:
+ DPRINTF("unrecognized request %0#2x\n", req);
...
Signed-off-by: Dov Murik <dovmurik@linux.vnet.ibm.com>
---
scripts/checkpatch.pl | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index bd3faa154c..6ec2a9f6a1 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2891,6 +2891,18 @@ sub process {
}
}
+# check for %# or %0# in printf-style format strings
+ while ($line =~ /(?:^|")([X\t]*)(?:"|$)/g) {
+ my $string = substr($rawline, $-[1], $+[1] - $-[1]);
+ $string =~ s/%%/__/g;
+ if ($string =~ /(?<!%)%0?#/) {
+ ERROR("Don't use '#' flag of printf format " .
+ "('%#') in format strings, use '0x' " .
+ "prefix instead\n" . $herecurr);
+ last;
+ }
+ }
+
# QEMU specific tests
if ($rawline =~ /\b(?:Qemu|QEmu)\b/) {
ERROR("use QEMU instead of Qemu or QEmu\n" . $herecurr);
--
2.20.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] checkpatch: Detect '%#' or '%0#' in printf-style format strings 2020-09-14 6:01 [PATCH] checkpatch: Detect '%#' or '%0#' in printf-style format strings Dov Murik @ 2020-09-14 6:55 ` Philippe Mathieu-Daudé 2020-09-14 11:27 ` Dov Murik 0 siblings, 1 reply; 5+ messages in thread From: Philippe Mathieu-Daudé @ 2020-09-14 6:55 UTC (permalink / raw) To: Dov Murik, qemu-devel, Eric Blake, Paolo Bonzini, Markus Armbruster +qemu-perl team On 9/14/20 8:01 AM, Dov Murik wrote: > According to the coding style document, we should use literal '0x' prefix > instead of printf's '#' flag (which appears as '%#' or '%0#' in the format > string). Add a checkpatch rule to enforce that. > > Note that checkpatch already had a similar rule for trace-events files. > > Example usage: > > $ scripts/checkpatch.pl --file chardev/baum.c > ... > ERROR: Don't use '#' flag of printf format ('%#') in format strings, use '0x' prefix instead > #366: FILE: chardev/baum.c:366: > + DPRINTF("Broken packet %#2x, tossing\n", req); \ > ... > ERROR: Don't use '#' flag of printf format ('%#') in format strings, use '0x' prefix instead > #472: FILE: chardev/baum.c:472: > + DPRINTF("unrecognized request %0#2x\n", req); > ... > > Signed-off-by: Dov Murik <dovmurik@linux.vnet.ibm.com> > --- > scripts/checkpatch.pl | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > index bd3faa154c..6ec2a9f6a1 100755 > --- a/scripts/checkpatch.pl > +++ b/scripts/checkpatch.pl > @@ -2891,6 +2891,18 @@ sub process { > } > } > > +# check for %# or %0# in printf-style format strings > + while ($line =~ /(?:^|")([X\t]*)(?:"|$)/g) { > + my $string = substr($rawline, $-[1], $+[1] - $-[1]); > + $string =~ s/%%/__/g; > + if ($string =~ /(?<!%)%0?#/) { > + ERROR("Don't use '#' flag of printf format " . > + "('%#') in format strings, use '0x' " . > + "prefix instead\n" . $herecurr); > + last; > + } > + } > + > # QEMU specific tests > if ($rawline =~ /\b(?:Qemu|QEmu)\b/) { > ERROR("use QEMU instead of Qemu or QEmu\n" . $herecurr); > Thank you for this patch! What about folding it in the same block? -- >8 -- --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2880,15 +2880,22 @@ sub process { $herecurr); } -# check for %L{u,d,i} in strings +# format strings checks my $string; while ($line =~ /(?:^|")([X\t]*)(?:"|$)/g) { $string = substr($rawline, $-[1], $+[1] - $-[1]); $string =~ s/%%/__/g; + # check for %L{u,d,i} in strings if ($string =~ /(?<!%)%L[udi]/) { ERROR("\%Ld/%Lu are not-standard C, use %lld/%llu\n" . $herecurr); last; } + if ($string =~ /(?<!%)%0?#/) { + ERROR("Don't use '#' flag of printf format " . + "('%#') in format strings, use '0x' " . + "prefix instead\n" . $herecurr); + last; + } } # QEMU specific tests --- ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] checkpatch: Detect '%#' or '%0#' in printf-style format strings 2020-09-14 6:55 ` Philippe Mathieu-Daudé @ 2020-09-14 11:27 ` Dov Murik 2020-09-14 13:00 ` Philippe Mathieu-Daudé 0 siblings, 1 reply; 5+ messages in thread From: Dov Murik @ 2020-09-14 11:27 UTC (permalink / raw) To: Philippe Mathieu-Daudé, qemu-devel, Eric Blake, Paolo Bonzini, Markus Armbruster On 14/09/2020 9:55, Philippe Mathieu-Daudé wrote: > +qemu-perl team > > On 9/14/20 8:01 AM, Dov Murik wrote: >> According to the coding style document, we should use literal '0x' prefix >> instead of printf's '#' flag (which appears as '%#' or '%0#' in the format >> string). Add a checkpatch rule to enforce that. >> >> Note that checkpatch already had a similar rule for trace-events files. >> >> Example usage: >> >> $ scripts/checkpatch.pl --file chardev/baum.c >> ... >> ERROR: Don't use '#' flag of printf format ('%#') in format strings, use '0x' prefix instead >> #366: FILE: chardev/baum.c:366: >> + DPRINTF("Broken packet %#2x, tossing\n", req); \ >> ... >> ERROR: Don't use '#' flag of printf format ('%#') in format strings, use '0x' prefix instead >> #472: FILE: chardev/baum.c:472: >> + DPRINTF("unrecognized request %0#2x\n", req); >> ... >> >> Signed-off-by: Dov Murik <dovmurik@linux.vnet.ibm.com> >> --- >> scripts/checkpatch.pl | 12 ++++++++++++ >> 1 file changed, 12 insertions(+) >> >> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl >> index bd3faa154c..6ec2a9f6a1 100755 >> --- a/scripts/checkpatch.pl >> +++ b/scripts/checkpatch.pl >> @@ -2891,6 +2891,18 @@ sub process { >> } >> } >> >> +# check for %# or %0# in printf-style format strings >> + while ($line =~ /(?:^|")([X\t]*)(?:"|$)/g) { >> + my $string = substr($rawline, $-[1], $+[1] - $-[1]); >> + $string =~ s/%%/__/g; >> + if ($string =~ /(?<!%)%0?#/) { >> + ERROR("Don't use '#' flag of printf format " . >> + "('%#') in format strings, use '0x' " . >> + "prefix instead\n" . $herecurr); >> + last; >> + } >> + } >> + >> # QEMU specific tests >> if ($rawline =~ /\b(?:Qemu|QEmu)\b/) { >> ERROR("use QEMU instead of Qemu or QEmu\n" . $herecurr); >> > > Thank you for this patch! > > What about folding it in the same block? > That makes sense, except that 'last' statement which will escape the loop if one of the bad patterns is found. Maybe we can just drop 'last' from both if-then blocks? We'll get multiple alerts if bad patterns are used more than once in the same line, which sounds OK to me. > -- >8 -- > --- a/scripts/checkpatch.pl > +++ b/scripts/checkpatch.pl > @@ -2880,15 +2880,22 @@ sub process { > $herecurr); > } > > -# check for %L{u,d,i} in strings > +# format strings checks > my $string; > while ($line =~ /(?:^|")([X\t]*)(?:"|$)/g) { > $string = substr($rawline, $-[1], $+[1] - $-[1]); > $string =~ s/%%/__/g; > + # check for %L{u,d,i} in strings > if ($string =~ /(?<!%)%L[udi]/) { > ERROR("\%Ld/%Lu are not-standard C, use > %lld/%llu\n" . $herecurr); > last; > } > + if ($string =~ /(?<!%)%0?#/) { > + ERROR("Don't use '#' flag of printf > format " . > + "('%#') in format strings, use > '0x' " . > + "prefix instead\n" . $herecurr); > + last; > + } > } > > # QEMU specific tests > --- > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] checkpatch: Detect '%#' or '%0#' in printf-style format strings 2020-09-14 11:27 ` Dov Murik @ 2020-09-14 13:00 ` Philippe Mathieu-Daudé 2020-09-22 9:22 ` Paolo Bonzini 0 siblings, 1 reply; 5+ messages in thread From: Philippe Mathieu-Daudé @ 2020-09-14 13:00 UTC (permalink / raw) To: Dov Murik, qemu-devel, Eric Blake, Paolo Bonzini, Markus Armbruster On 9/14/20 1:27 PM, Dov Murik wrote: > On 14/09/2020 9:55, Philippe Mathieu-Daudé wrote: >> +qemu-perl team >> >> On 9/14/20 8:01 AM, Dov Murik wrote: >>> According to the coding style document, we should use literal '0x' >>> prefix >>> instead of printf's '#' flag (which appears as '%#' or '%0#' in the >>> format >>> string). Add a checkpatch rule to enforce that. >>> >>> Note that checkpatch already had a similar rule for trace-events files. >>> >>> Example usage: >>> >>> $ scripts/checkpatch.pl --file chardev/baum.c >>> ... >>> ERROR: Don't use '#' flag of printf format ('%#') in format >>> strings, use '0x' prefix instead >>> #366: FILE: chardev/baum.c:366: >>> + DPRINTF("Broken packet %#2x, tossing\n", req); \ >>> ... >>> ERROR: Don't use '#' flag of printf format ('%#') in format >>> strings, use '0x' prefix instead >>> #472: FILE: chardev/baum.c:472: >>> + DPRINTF("unrecognized request %0#2x\n", req); >>> ... >>> >>> Signed-off-by: Dov Murik <dovmurik@linux.vnet.ibm.com> >>> --- >>> scripts/checkpatch.pl | 12 ++++++++++++ >>> 1 file changed, 12 insertions(+) >>> >>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl >>> index bd3faa154c..6ec2a9f6a1 100755 >>> --- a/scripts/checkpatch.pl >>> +++ b/scripts/checkpatch.pl >>> @@ -2891,6 +2891,18 @@ sub process { >>> } >>> } >>> +# check for %# or %0# in printf-style format strings >>> + while ($line =~ /(?:^|")([X\t]*)(?:"|$)/g) { >>> + my $string = substr($rawline, $-[1], $+[1] - $-[1]); >>> + $string =~ s/%%/__/g; >>> + if ($string =~ /(?<!%)%0?#/) { >>> + ERROR("Don't use '#' flag of printf format " . >>> + "('%#') in format strings, use '0x' " . >>> + "prefix instead\n" . $herecurr); >>> + last; >>> + } >>> + } >>> + >>> # QEMU specific tests >>> if ($rawline =~ /\b(?:Qemu|QEmu)\b/) { >>> ERROR("use QEMU instead of Qemu or QEmu\n" . $herecurr); >>> >> >> Thank you for this patch! >> >> What about folding it in the same block? >> > > That makes sense, except that 'last' statement which will escape the > loop if one of the bad patterns is found. > > Maybe we can just drop 'last' from both if-then blocks? We'll get > multiple alerts if bad patterns are used more than once in the same > line, which sounds OK to me. No objection. > > >> -- >8 -- >> --- a/scripts/checkpatch.pl >> +++ b/scripts/checkpatch.pl >> @@ -2880,15 +2880,22 @@ sub process { >> $herecurr); >> } >> >> -# check for %L{u,d,i} in strings >> +# format strings checks >> my $string; >> while ($line =~ /(?:^|")([X\t]*)(?:"|$)/g) { >> $string = substr($rawline, $-[1], $+[1] - >> $-[1]); >> $string =~ s/%%/__/g; >> + # check for %L{u,d,i} in strings >> if ($string =~ /(?<!%)%L[udi]/) { >> ERROR("\%Ld/%Lu are not-standard C, use >> %lld/%llu\n" . $herecurr); >> last; >> } >> + if ($string =~ /(?<!%)%0?#/) { >> + ERROR("Don't use '#' flag of printf >> format " . >> + "('%#') in format strings, use >> '0x' " . >> + "prefix instead\n" . $herecurr); >> + last; >> + } >> } >> >> # QEMU specific tests >> --- >> > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] checkpatch: Detect '%#' or '%0#' in printf-style format strings 2020-09-14 13:00 ` Philippe Mathieu-Daudé @ 2020-09-22 9:22 ` Paolo Bonzini 0 siblings, 0 replies; 5+ messages in thread From: Paolo Bonzini @ 2020-09-22 9:22 UTC (permalink / raw) To: Philippe Mathieu-Daudé, Dov Murik, qemu-devel, Eric Blake, Markus Armbruster On 14/09/20 15:00, Philippe Mathieu-Daudé wrote: >> That makes sense, except that 'last' statement which will escape the >> loop if one of the bad patterns is found. >> >> Maybe we can just drop 'last' from both if-then blocks? We'll get >> multiple alerts if bad patterns are used more than once in the same >> line, which sounds OK to me. > No objection. > Sounds good. Paolo ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-09-22 9:23 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-09-14 6:01 [PATCH] checkpatch: Detect '%#' or '%0#' in printf-style format strings Dov Murik 2020-09-14 6:55 ` Philippe Mathieu-Daudé 2020-09-14 11:27 ` Dov Murik 2020-09-14 13:00 ` Philippe Mathieu-Daudé 2020-09-22 9:22 ` Paolo Bonzini
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).