1. Patchew
  2. QEMU
  3. View series

[PATCH] checkpatch: Detect '%#' or '%0#' in printf-style format strings

Dov Murik posted 1 patch 3 years, 7 months ago
Test docker-quick@centos7 failed
Test docker-mingw@fedora failed
Test checkpatch passed
Test FreeBSD failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200914060109.69583-1-dovmurik@linux.vnet.ibm.com
There is a newer version of this series
scripts/checkpatch.pl | 12 ++++++++++++
1 file changed, 12 insertions(+)
[PATCH] checkpatch: Detect '%#' or '%0#' in printf-style format strings
Posted by Dov Murik 3 years, 7 months ago 
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
Re: [PATCH] checkpatch: Detect '%#' or '%0#' in printf-style format strings
Posted by Philippe Mathieu-Daudé 3 years, 7 months ago 
+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
---
Re: [PATCH] checkpatch: Detect '%#' or '%0#' in printf-style format strings
Posted by Dov Murik 3 years, 7 months ago 
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
> ---
>
Re: [PATCH] checkpatch: Detect '%#' or '%0#' in printf-style format strings
Posted by Philippe Mathieu-Daudé 3 years, 7 months ago 
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
>> ---
>>
>
Re: [PATCH] checkpatch: Detect '%#' or '%0#' in printf-style format strings
Posted by Paolo Bonzini 3 years, 7 months ago 
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

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.