* [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).